Bug 1452075 (CVE-2018-5158)

PDF Viewer will run code from PDF files, missing validation for /Domain and /Range parameters

VERIFIED FIXED in Firefox -esr52

Status

()

defect
P1
normal
VERIFIED FIXED
a year ago
8 months ago

People

(Reporter: gaubugzilla, Assigned: yury)

Tracking

({csectype-priv-escalation, sec-high})

Trunk
Firefox 61
Points:
---
Bug Flags:
sec-bounty +

Firefox Tracking Flags

(firefox-esr5260+ verified, firefox59 wontfix, firefox60+ verified, firefox61+ verified)

Details

(Whiteboard: [adv-main60+][adv-esr52.8+])

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
PDF Viewer will convert PostScript calculator functions to JavaScript. While the PostScript code itself is being validated thoroughly, for the Domain and Range parameters in the PDF file the code will only verify that these are arrays (see https://dxr.mozilla.org/mozilla-central/rev/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/build/pdf.worker.js#20413). PostScriptCompiler will then insert the array entries into the generated JavaScript code verbatim assuming that these are numbers (as they should be per spec), a malicious PDF file can use strings to inject JavaScript code this way.

Steps to reproduce:

1. Press Ctrl+Shift+J to open Browser Console.
2. Switch back to the browser and open the attached PDF file.
3. Check the Browser Console.

In Firefox 59.0.2 and 61.0a1 (2018-04-06) nightly the following message shows up in Browser Console: "Hello, this is code running in resource://pdf.js/build/pdf.worker.js." You can look at the text of the PDF file to see that the code being executed here is console.log("Hello, this is code running in " + location.href).

I did not implement additional exploit steps but code running in the worker can jump over to the viewer itself. For that it would send a malicious glyph to the viewer, you can see under https://dxr.mozilla.org/mozilla-central/rev/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/build/pdf.js#6354 that glyph data is being used to produce JavaScript code without further validation - this code assumes that validation happens in the worker.

Once in the viewer, the code could trigger the ChromeActions API available to the viewer (see https://dxr.mozilla.org/mozilla-central/rev/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/PdfStreamConverter.jsm#191) and read/write preferences in the pdfjs.* namespace for example. Also, it could add iframe tags pointing to other PDF files (e.g. from intranet domains). Since PDF Viewer would load into these domains as well, it could access their contents (same-origin) and steal PDF data.

Updated

a year ago
Flags: needinfo?(ydelendik)
Flags: needinfo?(bdahl)
Flags: sec-bounty?
...am I understanding it correctly, that the PDF viewer is able to bypass the Same Origin Policy?
Can someone confirm?
(Reporter)

Comment 2

a year ago
PDF Viewer can definitely bypass same-origin policy for PDF files (see the last two sentences in comment #0). I couldn't find a way to do it for arbitrary files however - ChromeActions are tied to the specific request URL, and while PDF Viewer could re-execute the constructor here, it can only determine the first parameter (it would need to manipulate the third).
(Assignee)

Comment 3

a year ago
(In reply to Wladimir Palant from comment #2)
> ChromeActions are tied to the specific request

Yes, we are keeping this invariant just for this reason.
Flags: needinfo?(ydelendik)
I'm assuming ESR52 is also affected until proven otherwise. Sounds like a sec-high at least since it doesn't seem like it would be overly difficult to get a user to open a booby-trapped PDF.
(Reporter)

Comment 5

a year ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
> since it doesn't seem like it would be overly difficult to
> get a user to open a booby-trapped PDF.

Given that PDF Viewer will apply to PDFs open in hidden iframes - no, getting the user to open the file is not a hurdle at all.
The fix for this is pretty trivial, but rolling it out is going to be a bit more complicated, as the PDF.js library has other consumers outside of Firefox. We'll need to coordinate updating pdf.js upstream, central, beta, and esr.
Flags: needinfo?(bdahl)
IIUC, Yury is working on this.
Assignee: nobody → ydelendik
Priority: -- → P1
This includes the fix for bug 1449898, which already has sec-approval. I'll be landing this under bug 1453838 with no reference to the security issues it's fixing. Brendan is working on rebasing this fix for Beta and ESR52 uplift.
Attachment #8967593 - Flags: sec-approval?
Comment on attachment 8967593 [details] [diff] [review]
[trunk] update pdf.js to version 2.0.491

sec-approval+.
Go and sin no more.
Attachment #8967593 - Flags: sec-approval? → sec-approval+
(Assignee)

Comment 10

a year ago
Posted patch patch for esr52 (obsolete) — Splinter Review
Attachment #8967593 - Attachment description: update pdf.js to version 2.0.491 → [trunk] update pdf.js to version 2.0.491
Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: sec-high
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: Brendan and Yury tested their fixes locally to confirm that the reported issue no longer reproduces and tests on CI are green (verified on Try).
[Needs manual test from QE? If yes, steps to reproduce]: Wouldn't be a bad idea. I did some basic smoketesting with the Try builds and didn't see any obvious issues, but more extensive testing for this and bug 1449898 wouldn't hurt.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: pdf.js has good automated test coverage in-tree and upstream. It's possible an edge case will break in an unanticipated way, but regular functionality should be fine
[String changes made/needed]: none
Attachment #8967896 - Flags: approval-mozilla-beta?
This is Yury's attached esr52 patch folded together with Brendan's patch from bug 1449898.
Attachment #8967852 - Attachment is obsolete: true
Attachment #8967897 - Flags: approval-mozilla-esr52?
Comment on attachment 8967896 [details] [diff] [review]
[beta] roll-up patch of bug 1449898 and bug 1452075

Approved for 60.0b13 and ESR 52.8.0.
Attachment #8967896 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8967897 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
https://hg.mozilla.org/mozilla-central/rev/676c6c0096af
https://hg.mozilla.org/releases/mozilla-beta/rev/ad91b3bde301
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Group: firefox-core-security → core-security-release
Flags: sec-bounty? → sec-bounty+
Whiteboard: [adv-main60+][adv-esr52.8+]
I have reproduced this bug using the STR from comment 0, on FF 59.0.2.

I can no longer see the "Hello, this is code running in resource://pdf.js/build/pdf.worker.js." message in the Browser console on 60.0a1 latest Nightly (2018-04-30) and 60.0b16 Beta (20180426170554) under the following OSes: Win 10 x64, macOS 10.13 and Ubuntu 16.04 x64.

Flagging myself as a reminder, to verify this on esr 52.8.0 as well.
Status: RESOLVED → VERIFIED
Flags: needinfo?(ciprian.georgiu)
Alias: CVE-2018-5158
This is also verified fixed on esr 52.8.0 (20180430140610) running Win 10 x64, Mac OS X 10.11 and Ubuntu 16.04 x64.
Flags: needinfo?(ciprian.georgiu)
When will CVE become public? FWIW, I'm still waiting on one more company to patch and they hope to have things fixed next week.
Flags: needinfo?(abillings)
(In reply to Brendan Dahl [:bdahl] from comment #19)
> When will CVE become public? FWIW, I'm still waiting on one more company to
> patch and they hope to have things fixed next week.

I don't see any embargo notation in this bug in regards to disclosure.

The advisory and CVE are scheduled to go live on Tuesday with the Firefox 60 release. Do we need to hold off on releasing this information?
Flags: needinfo?(abillings) → needinfo?(bdahl)
Whiteboard: [adv-main60+][adv-esr52.8+] → [Embargo? see comment 19+][adv-main60+][adv-esr52.8+]
Other company is all patched up. No need for an embargo.
Flags: needinfo?(bdahl)
Whiteboard: [Embargo? see comment 19+][adv-main60+][adv-esr52.8+] → [adv-main60+][adv-esr52.8+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.