1450443 - PDF Viewer should use a different principal (defense in depth)

Status: NEW

(Firefox :: PDF Viewer, enhancement, P3)

Description

[Wladimir Palant]

Right now, PdfStreamConverter.jsm will set the principal for the PDF Viewer to resource://pdf.js/. IMHO this is suboptimal because compared to a regular page from resource://pdf.js/ the PDF Viewer has special powers. Normally, an XSS in some page on resource:// wouldn't be a security issue - an attacker doesn't gain anything with it. If a webpage could inject JavaScript code into some page from resource://pdf.js/ however, it would be able to create a PDF Viewer frame which would be considered same-origin - meaning that it would gain access to PDF Viewer's special powers.

While this is not an issue right now (the only HTML page under resource://pdf.js/ is viewer.html, and an XSS vulnerability there would mean an XSS vulnerability in the PDF Viewer itself), using a different principal would eliminate that threat completely. For example, one could use resource://pdf.js.internal/ for the principal - there are no files under resource://pdf.js.internal/, so no issues to be exploited.

Comment 1

[:Gijs (he/him)]

(In reply to Wladimir Palant (for Adblock Plus info Cc [e-mail]) from comment #0)
> While this is not an issue right now (the only HTML page under
> resource://pdf.js/ is viewer.html, and an XSS vulnerability there would mean
> an XSS vulnerability in the PDF Viewer itself), using a different principal
> would eliminate that threat completely. For example, one could use
> resource://pdf.js.internal/ for the principal - there are no files under
> resource://pdf.js.internal/, so no issues to be exploited.

I don't think that in the current architecture it's easily possible to assign an arbitrary codebase principal to a document that doesn't match the document URI for the resulting page -- other than a null principal (which we use in a few places). I could be wrong, though. For a null principal, I'm not sure what you'd use for the origin string for postMessage, but I would be a little surprised if we could make that work. Part of the "point" of a null principal is that it's completely isolated from any other non-subsuming, non-system principal. Boris, do you have thoughts here?

FWIW, I'm also a bit confused what this suggestion is trying to protect against. My understanding of comment 0 is that it outlines:

1) we use resource://foo/, which means issues happen if anything (else) at resource://foo/ has XSS vulnerabilities, because it could x-origin access any pdf viewer it could get its hands on
2) there is only 1 document in resource://foo/ that would allow this to happen
3) that document is the pdf viewer, so if that had xss flaws we'd have an issue no matter what principal we gave the document.


As a principle (ba-dum-tss - I live in Britain now, so have to make at least 1 bad pun a day), it would be good to use a null principal. But the issue seems pretty theoretical. In terms of bang-for-our-buck, I'd be much more inclined, for instance, to try to lock down CSP on the pdf viewer if we haven't already... Brendan, do you know what the state of things is in that department? (that might want to be a separate bug if we do do something)

Comment 2

[Boris Zbarsky [:bzbarsky]]

It's not that hard to end up with a principal that doesn't match the document URI if you have access to the channel early enough.  That said, the mechanism one would use for that (the "owner") is something we are trying to remove.

In general, having a principal not matching the URI is not a great idea because not everything does security checks on principals...  So it's too easy to give yourself a false sense of security.

Comment 3

[Wladimir Palant]

I made this suggestion because the principal is being set explicitly by PdfStreamConverter.jsm in this case, it already doesn't match the document URI (the latter being the URI of the PDF file). Since the principal is chosen freely, it can be set to something that is more secure. But it is up to you of course to decide whether this is worth doing.

Comment 4

[Wladimir Palant]

For reference, a null principal would work in principle - it would make it rather hard to verify that messages from PdfStreamConverter.jsm are sent to the right document however (both postMessage() and custom events).

Comment 5

[Brendan Dahl [:bdahl]]

Clearing needinfo's that I'm unlikely to work on in the near future.