Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sandbox escape vulnerability #377

Closed
danielzgtg opened this issue Mar 4, 2023 · 12 comments
Closed

Sandbox escape vulnerability #377

danielzgtg opened this issue Mar 4, 2023 · 12 comments
Assignees
Labels
enhancement security Vulnerabilities or other issues affecting security upstream Should be worked on in kiwix-js

Comments

@danielzgtg
Copy link
Contributor

danielzgtg commented Mar 4, 2023

As mentioned in openzim/mwoffliner#1803 (comment) , I found a vulnerability in the sandbox that allows zims to break out and change the top level controls. The sandbox escape runs immediately after opening the zims.

This is new and don't involve redirects. Redirects nevertheless may weaken the sandbox, and I haven't checked whether CSP is enforced after redirects.

poc1

image

The fixed-position iframe stays there even after moving to the configuration page.

poc2

image

This one should be relatively simple to fix compared to poc1.

Steps

A zip of a proof-of-concept project is kiwix-pwa-poc.zip.

This the first time I have reported a security vulnerability ever in any project. I'm a bit lost and I don't see any SECURITY.md. I've read that it's not responsible to post proofs-of-concept publicly and that I should send an email instead. Therefore, I will be emailing [email protected] shortly with the password for the zip. EDIT: Email sent.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 4, 2023

@danielzgtg Thank you for reporting the vulnerability. While it is important to investigate, I should point out that the danger is low for these reasons:

  • Creating a ZIM is normally a specialist process, and they are made from known sites which are unlikely to be deploying an attack by using a ZIM
  • Kiwix JS PWA and Kiwix JS (upstream) run in browsers, so any vulnerability of the app itself will be gated by the browser itself, i.e. access to system files will not be possible

However, a vulnerability could be more serious with Electron and NWJS versions of these apps. I use the contextBridge mechanism in the Electron app, which is supposed to limit messaging between the browser/renderer process and the main process, so I believe this will protect users, but of course I'd like to examine the vulnerability you've identified.

If you use Slack and join the Kiwix Slack (open access), you could DM me there (same username). Otherwise by email egk10 (at) cam ac uk.

@danielzgtg
Copy link
Contributor Author

I agree that this vulnerability is not very serious. I and most people only use trusted zims from https://download.kiwix.org/ . After yesterday's research, I learned that I shouldn't open any random zims if anyone sends me one, and that should be enough.

the Kiwix Slack (open access),

I tried multiple times and multiple ways of joining the Kiwix Slack but couldn't. It says "The email address must match one of the domains listed below. Please try another email. You can use any account with the domain: kiwix.org mgautier.fr wikimedia.org marcelsuter.ch bibliosansfrontieres.org eduairbox.com banyanlabs.io Don’t have an email address from one of those domains? Contact the workspace administrator at Kiwix for an invitation." You could try inviting me with the email listed on my GitHub profile, though eventually this needs to be fixed for other people to make it actually "open access".

image

I think this means that you didn't receive the email with the password. I will forward it to the email you specified.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 4, 2023

Oops, sorry, I must have signed up with an invitation, I thought it was open! I've sent you an invite, in case interested, but email received, thanks.

@danielzgtg
Copy link
Contributor Author

I found the correct invitation at https://www.kiwix.org/en/support-us/code/ and will edit the page at https://wiki.kiwix.org/wiki/Communication . But I digress.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 5, 2023

In 8da7604 I'm testing the sandbox attribute as reported in kiwix/kiwix-js#972 (comment).

To test, visit the test PWA and let it update to v2.3.81 (after restart of app). So far, tested and working in Edge, Firefox and IE11.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 5, 2023

NB to test how this blocks top-level navigation in Wiktionary, it is necessary to turn off the option to Use locally cached Wikimedia styles (if it's on, then the vulnerability is patched by removing the offending script).

@Jaifroid
Copy link
Member

Jaifroid commented Mar 5, 2023

Working in:

  • Edge 17 on Windows 10 (though I wasn't able to test with Wikitionary, the most important thing is that having this sandbox attribute doesn't cause a regression)
  • Safari 16 on macOS Ventura
  • Safari 10.1 on macOS Sierra
  • Edge 15 on Windows Mobile
  • Samsung Internet on Android
  • Chrome on Android
  • Firefox on Android (with the Firefox file loading bug that copies file to memory)
  • IE11 on Windows 11

It should be noted that this is not a security fix against a malicious ZIM because, as Chrome warns:

image

But it is a useful extra safeguard against poorly coded scripts and accidental (or intentional) "phone-home" attempts.

@danielzgtg
Copy link
Contributor Author

I tested this to be working. It works as the controls survive both when the option is turned on and when it's turned off.

I tested Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/110.0 and Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/110.0.0.0 Safari/537.36.

as Chrome warns

Yeah, I see now. Firefox doesn't warn but when I loaded poc1 it still broke out, oh well. This should indeed block phone-home attempts by actors that don't expect to run in an iframe, which should include most accidental attempts. Actors that are aware they are inside an iframe can break out in the same way as poc1.

At least this fixes broken zims like wiktionary_en_all_maxi_2023-02.zim, which is what I think what a greater number of users care about.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 5, 2023

Thank you for testing. I opened the ZIMs you provided -- many thanks --, but I'm currently out of ideas to prevent these injections, unless you've had any further ideas. Possibly search for an additional iframe being added and halt the app with a warning. But adding an iframe is just one possible attack vector. It could be a div I guess, which I couldn't block. Or messing with the caches (possibly).

(The only other idea I had was to ask user to consider if they trust the source of a ZIM before allowing it to be opened.)

@Jaifroid Jaifroid self-assigned this Mar 5, 2023
@Jaifroid Jaifroid added enhancement upstream Should be worked on in kiwix-js security Vulnerabilities or other issues affecting security labels Mar 5, 2023
@Jaifroid
Copy link
Member

Jaifroid commented Mar 9, 2023

Update for this repo: Theoretically the vulnerability can be addressed by serving content to the iframe with a CSP header that includes the sandbox attribute. This is less vulnerable than using the iframe sandbox attribute alone. See kiwix/kiwix-js#753 (comment). I'll experiment with it on this Repo, and if it works, backport it upstream to Kiwix JS.

@Jaifroid
Copy link
Member

I think we've got as far as we can with this, and the fixes we have are included in v2.4.2 of the PWA. Consideration will be given to kiwix/kiwix-js#974, but for now I'm closing this.

@danielzgtg
Copy link
Contributor Author

I confirm that poc1 and poc2 are patched.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement security Vulnerabilities or other issues affecting security upstream Should be worked on in kiwix-js
Projects
None yet
Development

No branches or pull requests

2 participants