Skip to content
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

Eval Function("return this") is not secure #10818

Closed
Yunchitz opened this issue May 13, 2019 · 4 comments
Closed

Eval Function("return this") is not secure #10818

Yunchitz opened this issue May 13, 2019 · 4 comments

Comments

@Yunchitz
Copy link

Yunchitz commented May 13, 2019

Attach (recommended) or Link to PDF file here:

Configuration:

  • Web browser and its version:
  • Operating system and its version: WIN 10
  • PDFjs-dist version: 2.1.266
  • Is a browser extension:

Steps to reproduce the problem:

  1. Include lib to project with <meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline'>

What is the expected behavior? (add screenshot)
Please replace Function("return this") with not eval approach. Thanks a lot!
What went wrong? (add screenshot)

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

@Yunchitz Yunchitz changed the title Eval new Function("return this") is not secure Eval Function("return this") is not secure May 13, 2019
@Snuffleupagus
Copy link
Collaborator

Please replace Function("return this") with not eval approach.

This is done for performance reasons, and will consequently not be removed; as such this issue is INVALID.

However, if you look at the API documentation you'll see that this is already user-configurable when calling getDocument:

pdf.js/src/display/api.js

Lines 178 to 180 in fef86cc

* @property {boolean} isEvalSupported - (optional) Determines if we can eval
* strings as JS. Primarily used to improve performance of font rendering,
* and when parsing PDF functions. The default value is `true`.

@Yunchitz
Copy link
Author

Yunchitz commented May 13, 2019

It is happens only in two cases:
var g = function () {
return this;
}() || Function("return this")();
AND
(function () {
return this;
}() || Function("return this")());
/* WEBPACK VAR INJECTION */}.call(this, w_pdfjs_require(140)(module)))

May be it is happens before you add check isEvalSupported with try catch

Could you please help to fine a solution? We can't change the code of lib due to this is a npm module
Thank a lot!
Can I do like this
_pdfjs = require('pdfjs-dist');
_pdfjs.workerSrc = '../node_modules/pdfjs-dist/build/pdf.worker.js';
_pdfjs.isEvalSupported = false;

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented May 13, 2019

What's described in #10818 (comment) sounds like a duplicate of issue #10229 (which wasn't clear from the original comment above).

@timvandermeij
Copy link
Contributor

Yes, I would agree on the duplicate.

make-github-pseudonymous-again added a commit to infoderm/patients that referenced this issue May 11, 2024
By default, pdfjs-dist optimizes some path resolution logic by compiling
a JavaScript function on the fly. The function is built using string
concatenation and no effort is made at sanitizing the parts it is
built from. These parts could contain user-input which leads to a code
injection vulnerability. This commit disables this default behavior.
An alternative is to upgrade pdfjs-dist to v4.2.67 or later.

See:
  - https://bugzilla.mozilla.org/show_bug.cgi?id=1893645
  - https://www.cve.org/CVERecord?id=CVE-2024-4367
  - https://security.snyk.io/vuln/SNYK-JS-PDFJSDIST-6810403
  - GHSA-wgrm-67xf-hhpq.
  - mozilla/pdf.js#18015
  - wojtekmaj/react-pdf#1786
  - https://security.stackexchange.com/questions/248462/is-firefoxs-new-javascript-support-within-pdf-files-a-security-concern/248985#248985
  - https://stackoverflow.com/questions/49299000/what-are-the-security-implications-of-the-isevalsupported-option-in-pdf-js
  - mozilla/pdf.js#10818
make-github-pseudonymous-again added a commit to infoderm/patients that referenced this issue May 11, 2024
By default, pdfjs-dist optimizes some path resolution logic by compiling
a JavaScript function on the fly. The function is built using string
concatenation and no effort is made at sanitizing the parts it is
built from. These parts could contain user-input which leads to a code
injection vulnerability. This commit disables this default behavior.
An alternative is to upgrade pdfjs-dist to v4.2.67 or later.

For reference, see:
  - https://bugzilla.mozilla.org/show_bug.cgi?id=1893645
  - https://www.cve.org/CVERecord?id=CVE-2024-4367
  - https://security.snyk.io/vuln/SNYK-JS-PDFJSDIST-6810403
  - GHSA-wgrm-67xf-hhpq
  - mozilla/pdf.js#18015
  - wojtekmaj/react-pdf#1786
  - https://security.stackexchange.com/questions/248462/\
    is-firefoxs-new-javascript-support-within-pdf-files-a-security-concern/\
    248985
  - https://stackoverflow.com/questions/49299000/\
    what-are-the-security-implications-of-the-isevalsupported-option-in-pdf-js
  - mozilla/pdf.js#10818

Not sure if this will break anything and/or will make certain things
slower.
make-github-pseudonymous-again added a commit to infoderm/patients that referenced this issue May 11, 2024
By default, pdfjs-dist optimizes some path resolution logic by compiling
a JavaScript function on the fly. The function is built using string
concatenation and no effort is made at sanitizing the parts it is
built from. These parts could contain user-input which leads to a code
injection vulnerability. This commit disables this default behavior.
An alternative is to upgrade pdfjs-dist to v4.2.67 or later.

See:
  - https://bugzilla.mozilla.org/show_bug.cgi?id=1893645
  - https://www.cve.org/CVERecord?id=CVE-2024-4367
  - https://security.snyk.io/vuln/SNYK-JS-PDFJSDIST-6810403
  - GHSA-wgrm-67xf-hhpq.
  - mozilla/pdf.js#18015
  - wojtekmaj/react-pdf#1786
  - https://security.stackexchange.com/questions/248462/is-firefoxs-new-javascript-support-within-pdf-files-a-security-concern/248985#248985
  - https://stackoverflow.com/questions/49299000/what-are-the-security-implications-of-the-isevalsupported-option-in-pdf-js
  - mozilla/pdf.js#10818
github-merge-queue bot pushed a commit to infoderm/patients that referenced this issue May 11, 2024
By default, pdfjs-dist optimizes some path resolution logic by compiling
a JavaScript function on the fly. The function is built using string
concatenation and no effort is made at sanitizing the parts it is
built from. These parts could contain user-input which leads to a code
injection vulnerability. This commit disables this default behavior.
An alternative is to upgrade pdfjs-dist to v4.2.67 or later.

For reference, see:
  - https://bugzilla.mozilla.org/show_bug.cgi?id=1893645
  - https://www.cve.org/CVERecord?id=CVE-2024-4367
  - https://security.snyk.io/vuln/SNYK-JS-PDFJSDIST-6810403
  - GHSA-wgrm-67xf-hhpq
  - mozilla/pdf.js#18015
  - wojtekmaj/react-pdf#1786
  - https://security.stackexchange.com/questions/248462/\
    is-firefoxs-new-javascript-support-within-pdf-files-a-security-concern/\
    248985
  - https://stackoverflow.com/questions/49299000/\
    what-are-the-security-implications-of-the-isevalsupported-option-in-pdf-js
  - mozilla/pdf.js#10818

Not sure if this will break anything and/or will make certain things
slower.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants