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

Restrict what an iframe can do, for security reasons #753

Closed
mossroy opened this issue Aug 22, 2021 · 9 comments
Closed

Restrict what an iframe can do, for security reasons #753

mossroy opened this issue Aug 22, 2021 · 9 comments

Comments

@mossroy
Copy link
Contributor

mossroy commented Aug 22, 2021

This is a follow-up of #751

In case of a ZIM file with bad content in it, or a ZIM file that fetches bad content from the Internet, it could raise security issues.
Our browser extension does not ask for any permission in its manifest, but some nasty javascript could alter the content, send the articles read to an external server, break/alter our UI etc.

We should try to enforce that we only can execute code coming from the ZIM file (not from the Internet) and/or that the executed code is restricted to the iframe (no access to the UI).

Using the sandbox attribute might be enough. But it could also have bad consequences on the ability to open external links (see discussion in #404)

@Jaifroid
Copy link
Member

This specific post summarizes my research on this: #404 (comment).

To summarize the summary: we can block external resources from running by injecting a <meta> tag defining a CSP into the article before loading it into the iframe. To be clear, this alters the article. It's not a practical problem, but it could be a "philosophical" problem: should we alter documents for security reasons? Can we implicitly trust every ZIM published by Kiwix.org? Could a hacker insert content onto a site's pages which then gets incorporated "automatically" into a ZIM archive? What about ZIMs that users make with ZimIt (these don't currently work in Kiwix JS, I think)?

If we also want to block navigation to external resources, we would have to add a CSP to index.html (our own), but this produces a terrible UX IMHO. This is less of a security concern for us, I think.

@Jaifroid
Copy link
Member

There is a draft specification for enforcing a CSP on Embedded Content here: https://w3c.github.io/webappsec-cspee/

@danielzgtg
Copy link
Contributor

danielzgtg commented Mar 4, 2023

I no longer think it's possible to enforce sandbox restrictions on kiwix-js. I failed to do Object.freeze(window.location) and other things. To the browser, the kiwix-js controls look like just another script on the webpage. There's not really any practical way to tell the browser that the kiwix-js controls are more privileged and can control the zim scripts.

There are ways, but they have drawbacks:

  • Static analysis (a.k.a modifying the HTML before loading). Malicious zims could always obfuscate their content. One way I did so was using base64, but there are infinitely many other ways to scramble text. Therefore this would either be equivalent to solving the halting problem, or it would have to block good scripts.
  • Embed another Javascript engine using WASM. This would create a hard dependency on WASM, which old browsers don't have, and it will be slightly slow
  • Add a Javascript interpreter written in Javascript to emulate all the scripts. This will be very slow.

For both Javascript engine options, we would need to disable inline and eval scripts. Then only allowlisted DOM functions would be passed along. This will be slow or break a lot of zims unless compatibility is verified for each of them.

Therefore, I no longer think it's possible to completely secure the iframes while keeping the current quality of user experience.

EDIT:

implicitly trust every ZIM published by Kiwix.org

I don't see any signing of the zims. Apple signs iOS updates so that iPhones can only load unmodified software. I also don't want signing of the zims, because philosophically Wiki-related content should free as in freedom.

CSP on Embedded Content

What's the status on that proposal? I looked at https://github.com/w3c/webappsec and I don't know if it was abandoned or renamed.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 4, 2023

@danielzgtg Thanks for your testing on this. What I can (and do in the PWA, but clearly not 100% effective) is to add a CSP to the article loaded if it doesn't already have one and if the article is HTML-based. This was pretty essential to support Zimit archives, because they can easily end up loading external resources (it sometimes even happens on library.kiwix.org, you can be browsing a ZIM, and then suddenly realize you've been browsing an external site, loaded in the iframe and you never knew you'd entered it because it looks exactly the same).

However, adding the CSP was not done for security reasons, but to ensure that the app and its contents can only access local resources. Also, I allow a ZIM to have its own CSP if one is declared (so as not to break something that depends on its own CSP). I could review that. Of course it would never be 100% secure.

Therefore this would be equivalent to solving the halting problem

LOL, I like the analogy. Sometimes feels like developing a ZIM reader in JS is one big exercise in solving (and failing to solve) the halting problem... 🙂

But remember that ultimately Kiwix JS and Kiwix JS Windows/PWA are running in browsers, and malicious code in a ZIM can't do anything that a Web page also can't do. A good modern browser and framework should be pretty locked down. I just don't really see why anyone would use a ZIM as an attack vector when it would be much easier to mount an attack on the open Web with a much greater attack surface (in terms of visitors), and the ZIM attack isn't likely to achieve anything. If a user is going to be phished for their credit card details, it'll be much easier to phish them on the Web than in a ZIM...

The main reason in my mind to add CSP to pages is so that the user can be sure the ZIM will not "phone home" to its parent web site. This is important in places where accessing Wikipedia (for example) is heavily censored, though we also advise users to put their machine into offline mode in such scenarios.

@danielzgtg
Copy link
Contributor

I demonstrated in the other issue I created a zim with the "phone home" problem, namely to w3schools.com. Even for this use case, I think there is nothing bulletproof we can add to kiwix-js compared to just enabling airplane mode in the OS and unplugging the ethernet cable.

What I was originally thinking of was one zim affecting the display of another. The same-origin policy is the main security policy of the internet, and says that websites can do more or less whatever to themselves, but can do barely anything to other websites. Here, everything is loaded in the same pwa.kiwix.org or moz-extension.kiwix.org, so the browser thinks it's all one website and does not enforce any same-origin policy to separate zims. I can't think of any way to fix this. We could make subdomains and request browsers add us as an eTLD, but that defeats the purpose of kiwix working offline.

So the original attack I was thinking of goes like this. A malicious zim breaks out and gains the privileges of the controls. Optionally, it persists itself and disables updates, otherwise it'll have to settle for the user loading the zim every time. Then it intercepts the loading of every other zim that loads. Now it would be able to censor information, add misinformation/disinformation, or other harmful content like that kind of content that has increased in frequency and concern in the past few years. Normally a user would reasonably assume that the information shown on the screen would match the information stored in the zim on disk, and not depend on the zims opened previously. I suppose for this attack, a user could clear the data for the app (through the browser not the controls) after each zim, but that defeats the offline nature again.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 5, 2023

While it doesn't mitigate the issue of a seriously malicious script in a ZIM, the changes we made in #862 now make it possible to use the sandbox attribute of the iframe, which at least prevents the accidental "phone home" scenario, and also poorly coded scripts from breaking out of the iframe (i.e., accidental rather than malicious).

I have implemented the sandbox in the test PWA, and it does block top-level navigation and access to external resources by the iframe (such access is additionally blocked in the PWA by injecting a CSP into the article HTML). Here the first option (sandbox attribute) would be acceptable, but not the second (injecting CSP), as already discussed above.

I don't think we can do anything more in a JS reader. We could educate users, e.g. ask them if they trust the source of a ZIM before permitting them to load it, a bit like VS Code does when a user opens a new repository. I'll open a ticket for that.

@danielzgtg
Copy link
Contributor

danielzgtg commented Mar 5, 2023

ask them if they trust the source of a ZIM

This might scare a lot of users as non-programmers/non-jailbreakers don't usually see prompts like that. We could put a notice saying "Download trusted zims at https://www.kiwix.org/en/downloads/kiwix-content-packages/". That would also be useful for other reasons like bringing the similar feature from kiwix-js-windows here. But anything worded stronger would scare users unless it only appears for unofficial zims.

I'm slightly warming up to the idea of zim signing. This is the way it's like on Android, macOS, and Windows. Just don't completely block unsigned zims like iOS. For performance reasons since we deal with massive files, we would need something like dm-verity's use of Merkle trees.

@Jaifroid
Copy link
Member

Jaifroid commented Mar 9, 2023

There is another API I've come across that could prevent tampering by the document in the iframe: the CSP Sandbox, which can be turned on by serving the iframe contents with a sandbox header. We can set the header in the Service Worker before delivering the HTML of an article, and this should be tamper-proof.

@Jaifroid
Copy link
Member

I believe #976 takes us as far as it is possible to go with current technology, short of implementing all messages to the iframe via postMessage, which would be a major architectural shift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants