1452075 - (CVE-2018-5158) PDF Viewer will run code from PDF files, missing validation for /Domain and /Range parameters

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Wladimir Palant]

Attachment: Proof of concept PDF

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://hg.mozilla.org/mozilla-central/file/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/build/pdf.worker.js#l20413). 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://hg.mozilla.org/mozilla-central/file/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/build/pdf.js#l6354 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://hg.mozilla.org/mozilla-central/file/2f5ffe4fa2153a798ed8b310a597ea92abd1b868/browser/extensions/pdfjs/content/PdfStreamConverter.jsm#l191) 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.

Comment 1

[Frederik Braun [:freddy]]

...am I understanding it correctly, that the PDF viewer is able to bypass the Same Origin Policy?
Can someone confirm?

Comment 2

[Wladimir Palant]

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).

Comment 3

[Yury Delendik (:yury)]

(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.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 5

[Wladimir Palant]

(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.

Comment 6

[Brendan Dahl [:bdahl]]

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.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

IIUC, Yury is working on this.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: [trunk] update pdf.js to version 2.0.491

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.

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8967593 [details] [diff] [review]
[trunk] update pdf.js to version 2.0.491

sec-approval+.
Go and sin no more.

Comment 10

[Yury Delendik (:yury)]

Attachment: patch for esr52

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Trunk patch landed on inbound as part of bug 1453838.
https://hg.mozilla.org/integration/mozilla-inbound/rev/676c6c0096afa3e77d32ace4cfd4b21f65b53d15

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Attachment: [beta] roll-up patch of bug 1449898 and bug 1452075

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Attachment: [esr52] roll-up patch of bug 1449898 and bug 1452075

This is Yury's attached esr52 patch folded together with Brendan's patch from bug 1449898.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/676c6c0096af
https://hg.mozilla.org/releases/mozilla-beta/rev/ad91b3bde301

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/608e76ec5ba2

Comment 17

[Ciprian Georgiu, Desktop QA]

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.

Comment 18

[Ciprian Georgiu, Desktop QA]

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.

Comment 19

[Brendan Dahl [:bdahl]]

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.

Comment 20

[Al Billings [:abillings - ex-MoCo]]

(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?

Comment 21

[Brendan Dahl [:bdahl]]

Other company is all patched up. No need for an embargo.