Add `sandbox` attribute to the iframe generated by Kiwix Serve to prevent top-level navigation breaking out of the iframe (or malicious scripts)

[Jaifroid]

As reported by @danielzgtg, please see:

• https://github.com/kiwix/kiwix-js-windows/issues/377
• https://github.com/openzim/mwoffliner/issues/1803
• https://github.com/kiwix/kiwix-js/issues/753

For proof of the vulnerability, simply visit https://library.kiwix.org/content/wiktionary_en_all_nopic_2023-02/A/Wiktionary:Offline, and notice how a script in the latest Wiktionary attempts top-level navigation and breaks out of the iframe.

This should be preventable by adding the sandbox attribute to the iframe, e.g.:

sandbox="allow-same-origin allow-scripts"

Kiwix Serve probably does not need the other directives that KJSWL needs, such as allow-modals allow-forms allow-popups, but they may be needed for some Zimit ZIMs, so can be considered. Note that having allow-same-origin allow-scripts generates a warning in Chromium browsers:

If Kiwix Serve does not need to inject anything into the iframe, then maybe it can do away with allow-same-origin, and so harden security further. That is unfortunately not an option for Kiwix JS.

[danielzgtg]

If Kiwix Serve does not need to inject anything into the iframe, then maybe it can do away with allow-same-origin, and so harden security further. That is unfortunately not an option for Kiwix JS.

Removing allow-same-origin might indeed solve the rest of the problems. Where is Kiwix JS doing its injection, so we can analyze if such injection can be replaced? An alternative to whatever current injection is doing is to inject either everything we need or a stub in the service worker while loading each page in the iframe. Then we can use window.postMessage if there is any dynamic stuff left.

But again, there might not be much point in introducing that complexity either as it might not fix everything. The child iframe might still be able to mess with kiwix-appCache-2.4.0, and we will miss out on protecting newly opened tabs. EDIT: Nevermind. I tested a pair of html files, and removing allow-same-origin does indeed block access to caches.open from the iframe. This also works with 3 layers of iframes.

[Jaifroid]

Kiwix Serve doesn't use Kiwix JS under the hood, it merely creates an iframe so that it can show its toolbar. But the content can overwrite the iframe in the same way as in Kiwix JS and KJSWL, so there is a vulnerability. If there is no manipulation of DOM content in the iframe, and the whole page is simply recreated (with the iframe) on each page load, then it should be possible to use stronger sandboxing than in KJS.

[veloman-yunkan]

Note that having allow-same-origin allow-scripts generates a warning in Chromium browsers:

The documentation of <iframe>'s sandbox attribute contains the following note:

When the embedded document has the same origin as the embedding page, it is strongly discouraged to use both allow-scripts and allow-same-origin, as that lets the embedded document remove the sandbox attribute — making it no more secure than not using the sandbox attribute at all.

Unfortunately, my initial attempts to make the viewer work with only one of allow-scripts or allow-same-origin failed. Will have to dig deeper into it.

[veloman-yunkan]

I am also attaching a tiny ZIM-file demonstrating the problem: naughty.zim.zip

[Jaifroid]

Thanks @veloman-yunkan . I opened your naughty.zim in v2.4.0 of https://pwa.kiwix.org, which was "hardened" (insofar as is possible) yesterday, and got the following in console.log:

It was blocked from breaking out.

I agree that adding allow-scripts and allow-same-origin does not increase security against a malicious attacker, but it does prevent "accidental" attempts to break out of iframes. By accidental, I mean the accidental inclusion of frame-breaking scripts in a ZIM (clearly they should not be included), like in the latest English Wiktionary ZIM, or other problems with content that tries to navigate to top level.

Another thing that can be done is to inject a CSP into the incoming HTML. I do this currently in Kiwix JS PWA. This is what I inject:

<meta http-equiv="Content-Security-Policy" content="default-src 'self' data: blob: about: 'unsafe-inline' 'unsafe-eval';">

The aim of this is to prevent the iframe contents from accessing the Web at all (it effectively blocks such access). I'm not sure if it's any different from adding the sandbox on the iframe, but until yesterday I was only using this CSP.

[kelson42]

I guess there is nothing more we can do here? Should I close the ticket?

[Jaifroid]

Well there is one other possibility which might be more robust than setting the sandbox on the iframe. That would be to serve the content coming from the ZIM with a sandbox CSP header. See https://developer.mozilla.org/en-US/docs/web/http/headers/content-security-policy/sandbox . This would not be subject to tampering by the document itself.

I am thinking of this for Kiwix JS, as we can set the headers in the Service Worker. I guess this would be a separate issue, as this issue is about the iframe sandbox.

[kelson42]

@veloman-yunkan Any feedback on this? Should we implement it in kiwix-serve? All of this is pretty unclear yet to me (I don't invested enough time) but I'm not in favour of dynamically modifying the HTML at each loading for this reason.

[veloman-yunkan]

@veloman-yunkan Any feedback on this? Should we implement it in kiwix-serve? All of this is pretty unclear yet to me (I don't invested enough time) but I'm not in favour of dynamically modifying the HTML at each loading for this reason.

@kelson42 We don't have to modify the HTML - we can set a HTTP header in our responses.

[Jaifroid]

Agreed, this is not about modifying HTML, only setting the response header. I haven't tested this solution, but if it works, it would probably be the most robust solution for security (additional to having the iframe sandbox).

[Jaifroid]

OK, I've now tested this in Kiwix JS PWA code. I removed the iframe sandbox and instead added a Content-Security-Policy response header in the Service Worker (I haven't commited this code yet, as I'm experimenting). I observed the network requests and responses. The results are as below. You can see the response header in top-right box, but you can also see the warning generated by Chromium (bottom pane, gold-coloured text): An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can remove its sandboxing.

The question is, is this true or is this just a generic warning? Remember, the iframe no longer has a sandbox attribute (I've double-checked it's no longer there). So how can a document in the iframe alter its own response header which is sent before it is even loaded? It surely can't rewrite the server or the Service Worker. Maybe it can search through the parent elements of the iframe until it finds a div that it can fill with spammy content?

---

The question to decide here is whether setting the headers is more secure than using the sandbox attribute of the iframe, or whether we should have both for good measure.

[Jaifroid]

And this doesn't prevent @danielzgtg's poc1 ZIM attack (here, an additional iframe has been added by the content in the iframe at the top level, and it has loaded third-party content).

[Jaifroid]

I can tighten security somewhat by specifying frame-src directive in CSP. In this case, using the meta http-equiv tag in the top-level document, like so:

<meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline' 'unsafe-eval'; frame-src 'self' file: data: blob: 'unsafe-inline' 'unsafe-eval';">

This at least prevents the poc1.zim from loading the third-party image, but doesn't prevent it creating a top-level iframe:

[veloman-yunkan]

Unconditional sandboxing (due to kiwix/libkiwix#906) effectively blocks navigating to external webpages even in a kiwix-serve instance started without the --blockexternal option - clicking external links is silently ignored.

[Jaifroid]

We solved that in Kiwix JS by registering a click on the iframe and inspecting it. Will find the code when I get back to a PC.

[Jaifroid]

OK, relevant code is here: https://github.com/kiwix/kiwix-js/blob/main/www/js/app.js#L1587. Transcribed:

                        // Add event listener to iframe window to check for links to external resources
                        iframeArticleContent.contentWindow.addEventListener('click', function (event) {
                            // Find the closest enclosing A tag (if any)
                            var clickedAnchor = uiUtil.closestAnchorEnclosingElement(event.target);
                            if (clickedAnchor) {
                                var href = clickedAnchor.getAttribute('href');
                                // We assume that, if an absolute http(s) link is hardcoded inside an HTML string,
                                // it means it's a link to an external website.
                                // We also do it for ftp even if it's not supported any more by recent browsers...
                                if (/^(?:http|ftp)/i.test(href)) {
                                    uiUtil.warnAndOpenExternalLinkInNewTab(event, clickedAnchor);
                                }
                                if (/\.pdf$/i.test(href)) {
                                    // Due to the iframe sandbox, we have to prevent the PDF viewer from opening in the iframe and instead open it in a new tab
                                    event.preventDefault();
                                    window.open(clickedAnchor.href, '_blank');
                                }
                            }
                        });

iframeArticleContent is simply the iframe selected by its id. And in case you need it, closestAnchorEnclosingElement is here: https://github.com/kiwix/kiwix-js/blob/main/www/js/lib/uiUtil.js#L637.

[danielzgtg]

There is allow-popups and possibly allow-popups-to-escape-sandbox to add inside sandbox="...". target="_blank" might need to be injected onto all links. Haven't confirmed, but someone could explore if this works.

[Jaifroid]

In my experience, allow-popups is not the same as opening in a completely new window or tab. While I do allow-popups on Kiwix JS PWA, because it allows for the "Open link in new browsable tab / window" feature, it's probably not needed for Kiwix Serve. Having target="_blank" is sufficient for opening external web sites and PDFs.

FWIW, I think allow-popups-to-escape-sandbox is a security and privacy risk.

[danielzgtg]

I agree for allow-popups-to-escape-sandbox. It might also allow target="_blank" to preexisting internal links in the zim. However, I thought we decided on just adding a dialog to ask the user to confirm before opening a new zim. allow-same-origin seems too big of a change to remove, and security is impossible with it. Like the Chrome warning says, the zim's script can just go and add allow-popups-to-escape-sandbox itself, making it irrelevant for security whether you add it or not. At this point, unless someone has massive amounts of time for MessageChannel, the only job the sandbox does well and can do at all is fixing the redirecting zims.

[kelson42]

@veloman-yunkan @Jaifroid @danielzgtg Can we close that one now?

[Jaifroid]

Yes.