-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Prevent cross-origin information leakage via range requests #12744
Comments
Just asking, but shouldn't this include https://github.com/mozilla/pdf.js/blob/master/src/display/fetch_stream.js and https://github.com/mozilla/pdf.js/blob/master/src/display/node_stream.js as well? |
Yes. I didn't enumerate everything, but just some of the examples (hence "at least in the following places"). |
For MC, I think we can disable redirects e.g. https://searchfox.org/mozilla-central/rev/8698fade12984b9a6a43a85a287a5f17e8fd4ddf/toolkit/components/captivedetect/CaptiveDetect.jsm#46 |
Disabling redirects is a way to resolve this issue, but only if we are absolutely sure that it is safe to do so. |
If we can disable it and prevent this potential security issue, please disable it and land the patch. |
Unless I'm missing something, I believe this is how chrome fixed it. |
That is safe (i.e. not breaking legitimate use cases) IF the PDF resource URL is the final URL. If that is the case, simply blocking redirects can fix this issue. |
FYI I've also reported this with more details at https://bugzilla.mozilla.org/show_bug.cgi?id=1683940 which was associated with CVE-2021-23953. This issue was fixed in Firefox, without any code changes in PDF.js because the privileged viewer integration code is fully contained in Firefox's code base. The Chrome extension (and any other environment without same origin policy) is still vulnerable to this issue. |
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
Regression tests for issue mozilla#12744 and PR mozilla#19028
(this is a follow-up to https://phabricator.services.mozilla.com/D91746#3198225)
PDF.js does currently not validate the origin of a PDF response before using it. In web pages, this is not a problem, because unauthorized access is blocked by the same-origin policy. In privileged contexts, such as a browser feature or a browser extension, this can become problematic as soon as it is possible to read content from a PDF file into a web page. Even a unidirectional channel with a binary value (e.g. a boolean yes or no) is enough to read information. With the ongoing work for scripting support (relevant components linked from #12689 (review)), the condition for exploitation is about to be met.
In pages 12 and 13 of https://robwu.nl/s/bugswat2019rw.pdf#page=12, I sketched an attack that abuses Range requests to stitch together responses from different origins to read data across origins, with a proof-of-concept exploit that targeted Chrome / PDFium (CVE-2016-5206).
PDF.js does currently not validate the origins/URLs of responses either - we should do that to prevent a similar attack from happening, at least in the following places:
src/display/network.js
We should determine the origin of the PDF file (e.g. from the first response) and then verify that the origin is consistent (and otherwise reject the response).
xhr.responseURL
can be used to obtain the final response URL, after all redirections. Iffetch
is used,response.url
has the same information.If we are concerned about potentially breaking functionality for users, we can start by adding telemetry to see how often the origin of the initial response differs from the origin of subsequent requests, before we start to enforce the new restriction.
The text was updated successfully, but these errors were encountered: