1893645 - (CVE-2024-4367) Arbitrary Javascript injection in PDF.js through FontMatrix

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Thomas Rinsma]

Attachment: js_injection_poc.pdf

(Tested with Firefox 124.0b9 on Ubuntu and with PDF.js latest from Git.)

Arbitrary Javascript code can be executed in the PDF.js context (unrelated to the Javascript sandbox) by abusing a lack of type checking in the glyph path compilation process, specifically when a Type 1 font is involved. A prerequisite is that isEvalSupported needs to be true (it is by default).

This results in a somewhat universal XSS when a PDF is opened with Firefox (cross-origin restrictions are still in place). An attacker can control the PDF.js window and do things like spy on the user’s activity, trigger downloads using pdf.js.message events (even file:// URLs can be “downloaded” in this way) or leak the PDF’s local file path from window.PDFViewerApplication.url.

In the cases where PDF.js is used in a web-application, this leads to a stored XSS attack on the respective page’s origin.

Details

The method FontFaceObject.getPathGenerator(...) compiles glyphs to Javascript functions that will return the associated path. In case isEvalSupported is set to true, it does this by concatenating them into a list of Javascript statements and creating a Function object out of that: (https://github.com/mozilla/pdf.js/blob/90d4b9c2c0df7cadbd5c2388431b208f08da1ed6/src/display/font_loader.js#L450 )

    // If we can, compile cmds into JS for MAXIMUM SPEED...
    if (this.isEvalSupported && FeatureTest.isEvalSupported) {
      const jsBuf = [];
      for (const current of cmds) {
        const args = current.args !== undefined ? current.args.join(",") : "";
        jsBuf.push("c.", current.cmd, "(", args, ");\n");
      }
      // eslint-disable-next-line no-new-func
      return (this.compiledGlyphs[character] = new Function(
        "c",
        "size",
        jsBuf.join("")
      ));
    }

This list of commands, cmds, is generated by the method CompiledFont.compileGlyph(...) and the compileGlyphImpl variants for the different font types: (trimmed from https://github.com/mozilla/pdf.js/blob/90d4b9c2c0df7cadbd5c2388431b208f08da1ed6/src/core/font_renderer.js#L798 )

  compileGlyph(code, glyphId) {
    ...
    let fontMatrix = this.fontMatrix;
    ...
    const cmds = [
      { cmd: "save" },
      { cmd: "transform", args: fontMatrix.slice() },
      { cmd: "scale", args: ["size", "-size"] },
    ];
    this.compileGlyphImpl(code, cmds, glyphId);

    cmds.push({ cmd: "restore" });

    return cmds;
  }

For the arguments of the transform command it takes the fontMatrix array from the current font object. It turns out that under specific circumstances (seemingly only for Type1 fonts) this array is grabbed from a PDF dictionary and not (also) from the font data itself. Hence, the array can contain arbitrary PDF primitives including strings. The retrieval happens in PartialEvaluator.translateFont:

    const properties = {
      ...
      fontMatrix: dict.getArray("FontMatrix") || FONT_IDENTITY_MATRIX,
      ...
    };

(Note: with other font types this does not appear to be a problem as the FontMatrix is later grabbed from the font data itself (e.g. CFF dicts or Type1 “PostScript” parsing) where the parsers explicitly expect numeric array elements. Even if it is not present in the font data, it is overwritten with a standard matrix.)

This means that string-type elements get concatenated literally in the resulting Javascript. Hence we can craft a PDF with a Type1 font and a Font object with this FontMatrix definition;

  /FontMatrix [1 2 3 4 5 (6\); alert\('foobar')]

resulting in this Javascript code being executed when the glyph is rendered:

c.save();
c.transform(1,2,3,4,5,6); alert('foobar');
c.scale(size,-size);
c.moveTo(0,0);
c.restore();

Attached is a plain-text minimal PoC.

Possible solution

Type-validation could be added either in compileGlyph, or in translateFont. In the latter case, maybe it is beneficial to implement something like dict.getNumericArray in addition to dict.getArray for situations like this.

Comment 1

[Calixte Denizet (:calixte)]

Thank you for this bug report.

Comment 2

[Calixte Denizet (:calixte)]

Here's a patch:
https://github.com/calixteman/pdf.js/commit/bcbc9615d1867032c70807607752c233cc650807
:Snuffleupagus, could you review it please ?

Comment 3

[Jonas Jenwald [:Snuffleupagus]]

The reason that eval is being used is to improve performance during fallback glyph-rendering, how does removing that affect things?
Please share some benchmarking data for relevant PDFs with embedded fonts, using disableFontFace = true to trigger this code-path.

Comment 4

[Calixte Denizet (:calixte)]

In term of performance, it's almost the same.
I got two profiles in dev edittion 126 with a local setup and in using http://localhost:8888/web/viewer.html?file=/test/pdfs/tracemonkey.pdf#zoom=1

• with the patch: https://profiler.firefox.com/public/e8twyj8xsy9k3kmm9b7h6e0a284h4m1jf3w2sr8/calltree/?globalTrackOrder=0w2&implementation=js&thread=3&timelineType=category&v=10
  235ms is spent in glyphDrawer
• without the patch: https://profiler.firefox.com/public/1xccy1wvhm8fc9res7s0v8kmn338eyyd6dw810r/calltree/?globalTrackOrder=0w2&implementation=js&thread=3&v=10
  251ms is spent in anonymous.

Comment 5

[Donal Meehan [:dmeehan]]

:marco/:calixte what's the plan for uplifting this for beta and esr115?
Are you going to attach a patch that we can uplift?

Comment 6

[Andrew McCreight [:mccr8]]

sec-high bugs that affect more than Nightly should get sec-approval before landing.
See: https://firefox-source-docs.mozilla.org/bug-mgmt/processes/security-approval.html

Comment 7

[Thomas Rinsma]

Thanks all for picking this up so fast!

I see that v4.2.67 of PDF.js (and hence pdfjs-dist on NPM) has been released containing the fix, but without an explicit security advisory. This means that users of the package (and its wrappers like react-pdf) will not be warned via services like npm audit and Snyk. Maybe this can still be done? I am not entirely sure how that process works to be honest, maybe the easiest is via a Github security advisory?

I was also about to request a CVE but noticed that Mozilla is a numbering authority itself. Let me know if there's still something I can help with there.

Comment 8

[Calixte Denizet (:calixte)]

:dmeehan, yes I'll attach some patches today.
:mccr8, I'm sorry about that.
:Thomas, I don't know, probably :dveditz can answer.

Comment 9

[Calixte Denizet (:calixte)]

Attachment: Bug 1893645 - Simplify the way to pass font data to the main thread

Comment 10

[Calixte Denizet (:calixte)]

Attachment: Bug 1893645 - Simplify the way to pass font data to the main thread

Comment 11

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Some arbitrary js can be executed within the viewer
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Open the pdf attached to the bug and no popup should be triggered
• Risk associated with taking this patch: low
• Explanation of risk level: It's a self contained change and usually fonts aren't rendered in using the specific pdf.js font render but in using the Firefox one
• String changes made/needed: No
• Is Android affected?: yes

Comment 12

[Phabricator Automation]

esr115 Uplift Approval Request

• User impact if declined: Some arbitrary js can be executed within the viewer
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Open the pdf attached to the bug and no popup should be triggered
• Risk associated with taking this patch: low
• Explanation of risk level: It's a self contained change and usually fonts aren't rendered in using the specific pdf.js font render but in using the Firefox one
• String changes made/needed: No
• Is Android affected?: yes

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/32756264a810

Comment 14

[Daniel Veditz [:dveditz]]

This is worse for website-hosted PDF.js (XSS) but that's not covered by our bounty program. There doesn't seem to be a UXSS risk here, but the resource:// privileged context would be able to navigate to URLs that web content is blocked from (chrome:// urls with arbitrary parameters, file:// urls). You could easily trigger a download and save using PDFjs capabilities, and then open the attackers content as a file:// URL. If there was a content-process RCE bug that would get the attacker into a process with a weaker sandbox, giving access to the entire file system.

Comment 15

[Tom Ritter [:tjr]]

(In reply to Thomas Rinsma from comment #7)

Thanks all for picking this up so fast!

I see that v4.2.67 of PDF.js (and hence pdfjs-dist on NPM) has been released containing the fix, but without an explicit security advisory. This means that users of the package (and its wrappers like react-pdf) will not be warned via services like npm audit and Snyk. Maybe this can still be done? I am not entirely sure how that process works to be honest, maybe the easiest is via a Github security advisory?

I was also about to request a CVE but noticed that Mozilla is a numbering authority itself. Let me know if there's still something I can help with there.

Yes, an advisory should be released via both Github and the normal advisory process. CVE-2024-4367 can be used for the Github Advisory; calixte if you have trouble writing the advisory, feel free to reach out in #secinf on Slack

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/710a50f0f51d

Comment 17

[:Gijs (he/him)]

At a higher level I wonder if we can use Cu.sandbox() for the generated JS bits, which would at least have contained the vulnerability here. Is there a plan for this or something similar here to defend-in-depth a bit more?

(I seemingly neglected to send this comment a few days ago, before the patch went up - I see now that the patch removed this particular use of eval entirely, but on m-c in searchfox I still see an isEvalSupported property - are there other places where eval or new Function or similar is used?)

Comment 18

[Calixte Denizet (:calixte)]

We still use new Function in:
https://github.com/mozilla/pdf.js/blob/master/src/core/function.js#L428-L436
I audited this code few days ago and I didn't see anything wrong, the generated function is executed in the worker so there's no access to the DOM.
eval is used here in order to improve performance of some postscript functions, so I'm not sure that sending the execution in a sandbox and adding a sync point would help to keep the same performance.

Comment 19

[Tom Ritter [:tjr]]

We've published an advisory at https://github.com/mozilla/pdf.js/security/advisories/GHSA-wgrm-67xf-hhpq - we will write a similar advisory for Firefox explaining its impact in that context and use the same CVE.

Comment 20

[Ciprian Georgiu, Desktop QA]

I have reproduced this bug using an affected Nightly 127.0a1 (2024-04-26) on macOS 13 with STR from comment 12.

The issue is verified as fixed on latest Nightly 127.0a1, Release 126.0 and Esr 115.11.0 under Win 11 x64, macOS 13 and Ubuntu 20.04.

Comment 21

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt

Comment 22

[Thomas Rinsma]

:truber, not sure about any policies but would it be possible to add my organization name ("Codean Labs") to the credit in the advisory?

Comment 23

[Jesse Schwartzentruber (:truber)]

Attachment: advisory.txt

(In reply to Thomas Rinsma from comment #22)

:truber, not sure about any policies but would it be possible to add my organization name ("Codean Labs") to the credit in the advisory?

Done

Comment 24

[:Gijs (he/him)]

(In reply to Calixte Denizet (:calixte) from comment #18)

We still use new Function in:
https://github.com/mozilla/pdf.js/blob/master/src/core/function.js#L428-L436
I audited this code few days ago and I didn't see anything wrong, the generated function is executed in the worker so there's no access to the DOM.
eval is used here in order to improve performance of some postscript functions, so I'm not sure that sending the execution in a sandbox and adding a sync point would help to keep the same performance.

A gecko sandbox does not affect performance of the code running inside the sandbox at all - it ends up running the same JS but in a different scope / with a different level of privilege. The only thing that would potentially be affected would be the marshalling of complex things into and out of the sandbox (as they'd have to be structuredClone'd or use xpc wrappers), but that seems less likely to be an issue here as it looks like these postscript functions deal with numbers only? I'm not a JS engine expert but I would expect those to be performant just fine. Can we file a follow-up bug to investigate and get someone on the JS team to weigh in? Maybe it ends up wontfix if we really think the trade-off isn't worth it, but it seems worth at least checking if it could work. :-)

Comment 25

[Calixte Denizet (:calixte)]

That's fair:
https://bugzilla.mozilla.org/show_bug.cgi?id=1896200

Comment 26

[Calixte Denizet (:calixte)]

A gecko sandbox does not affect performance of the code running inside the sandbox at all

I know it's possible to have a sandbox outside a worker and then my point was about having a sync point in the worker waiting for the results of the computations. But if it's possible to have a sandbox in the worker, then it could be interesting.

Comment 27

[Rob Wu [:robwu]]

The PDF.js advisory at https://github.com/mozilla/pdf.js/security/advisories/GHSA-wgrm-67xf-hhpq
currently shows "Affected versions <= 4.1.392". Since PDF.js as a library is used by third-party websites which would be impacted more severely than Firefox, I wondered whether this is a recent regression and checked the affected versions. Other than the latest release, there a period in 2016-2017 where the bug was absent.

PDF.js versions and statuses:

• v4.2.67 (released April 29, 2024) unaffected (fixed)
  • Commit: https://github.com/mozilla/pdf.js/commit/85e64b5c16c9aaef738f421733c12911a441cec6
  • Pull request: https://github.com/mozilla/pdf.js/pull/18015
• v4.1.392 (released April 11, 2024) affected (release before this bug was fixed)
• v1.10.88 (released Oct 27, 2017) affected (re-introduces the security vulnerability due to a typo fix)
  • Commit: https://github.com/mozilla/pdf.js/commit/ab4d5be19294e3dc2972efd07d1c1373cc8e7e85
  • Pull request: https://github.com/mozilla/pdf.js/pull/9008
• v1.9.426 (released Aug 15, 2017) unaffected (release before the next affected version)
• v1.5.188 (released Apr 21, 2016) unaffected (mitigated the security vulnerability by an accidental typo)
  • commit: https://github.com/mozilla/pdf.js/commit/1d12aed5cac6a0caccb0550e0aad4ef9f6bb263d
  • Pull request: https://github.com/mozilla/pdf.js/pull/7126 - the typo is later fixed by PR 9008.
• v1.4.20 (released Jan 27, 2016) affected (release before the next release that accidentally fixed the vulnerable code)
• v0.8.1181 (released Apr 10, 2014) affected (first public release of PDF.js)
  • Commit: https://github.com/mozilla/pdf.js/commit/0e133f00902b9fef8422a932385a61a38ca85545
  • Pull request: https://github.com/mozilla/pdf.js/pull/3195

Note: some may use the master branch of PDF.js instead of specific releases. Firefox is an example:

• Firefox 58 - affected, PDF.js version 2.0.106
• Firefox 57 - unaffected, PDF.js version 1.9.583

Comment 28

[Rob Wu [:robwu]]

Attachment: poc_CVE-2024-4367.pdf

The original PoC (js_injection_poc.pdf) did not reproduce the vulnerability in older PDF.js versions. Here is an updated PoC.

There are three changes:

• The dictionary at 7 0 obj was modified; I added /Length1 2462
• The JS snippet was modified; I added a conditional to fall back to document.URL if window.PDFViewerApplication is undefined.
• I recalculated the offsets in the Xref table (qpdf js_injection_poc.pdf poc_CVE-2024-4367.pdf)

To test whether a PDF.js version is vulnerable, open the PDF file in the viewer. If vulnerable, an alert dialog will pop up as seen in the original report.

Note: the absence of the dialog does not imply a non-vulnerable PDF.js installation. For example, if the PDF Viewer is loaded in a iframe sandbox or CSP sandbox without allow-modals, then the dialog does not pop up. For a version-based vulnerability assessment, see https://bugzilla.mozilla.org/show_bug.cgi?id=1893645#c27

Comment 29

[Thomas Rinsma]

Thank you Rob for the thorough analysis. Given that this may be useful for people externally as well, do you mind if I add the version overview and/or generalized PoC to the public blog post?

Comment 30

[Rob Wu [:robwu]]

I have posted the same version overview at https://github.com/mozilla/pdf.js/pull/18015#issuecomment-2124596367, along with a remark that "unaffected" does not mean that these versions are safe to use. That is because these older versions are affected by a different vulnerability (bug 1452075 aka CVE-2018-5158).

Feel free to share my PoC (and name). For context, the old PoC did not work in PDF.js<=1.4.20 despite it being vulnerable, the new one does.
Since your blog has already been shared, it is likely that the PoC has already been copied by others. I therefore recommend to clearly state that the PoC has been updated to cover older versions, in case people come across your original PoC and wonder why it didn't reproduce despite allegedly being vulnerable.

Comment 31

[Thomas Rinsma]

Clear, thanks for the guidance. I've added the information.

Comment 33

[Daniel Veditz [:dveditz]]

No point in keeping this hidden given the reporter's blog post