diegomura / react-pdf

📄 Create PDF files using React
https://react-pdf.org
MIT License
14.34k stars 1.13k forks source link

Vulnerability in used pdfjs-dist version #2746

Open Sogeman opened 1 month ago

Sogeman commented 1 month ago

Describe the bug

from npm audit

pdfjs-dist <=4.1.392 Severity: high PDF.js vulnerable to arbitrary JavaScript execution upon opening a malicious PDF - https://github.com/advisories/GHSA-wgrm-67xf-hhpq

Is this something you can change here or is it further up the chain?

Edit: I see dependabot already added a PR for that

thanks

marvinjaworski commented 1 month ago

Same problem here, we are very close to a release with our software and this bug is a big problem for our compliance regulations. It would be important to fix this quickly please 🙏

donovanclarke commented 1 month ago

Just another engineer chiming in on this 🙏🏽

Edit:

You could possibly use a resolution as a workaround. I will be testing this shortly.

bombillazo commented 1 month ago

Updated my packages and got this warning as well.

davidovich9 commented 1 month ago

Temporary fix that worked for me:

  "resolutions": {
    "pdfjs-dist": "^4.2.67"
  }
    optimizeDeps: {
        esbuildOptions: {
            target: 'esnext'
        }
    },
    build: {
        target: 'esnext'
    }
marvinjaworski commented 1 month ago

@davidovich9 I use react without nextjs and vite. When I set the resolution in the package.json, the message "2 high severity vulnerabilities" persists on npm install.

donovanclarke commented 1 month ago

@davidovich9 I use react without nextjs and vite. When I set the resolution in the package.json, the message "2 high severity vulnerabilities" persists on npm install.

Are you using yarn, or npm?

yarn you can use resolutions in your package.json file.

And i believe the npm equivalent is overrides.

bombillazo commented 1 month ago

Using overrides worked for us to remove the warning! What exactly is that doing to fix the issue?

donovanclarke commented 1 month ago

Using overrides worked for us to remove the warning! What exactly is that doing to fix the issue?

Say you have package A.

And package B, C, D use A as a dependency, but they all use different versions of A.

A resolution or override basically centralizes that version to what you have in your resolution or override.

IMHO, its not a long term solution, but definitely can help out in times like this when we need to quickly get something out.

grueneerle commented 1 month ago

The temporary override (pdfjs-dist -> 4.2.67) seems to fix the audit issues but we (@marvinjaworski ) are facing compatibility issues withreact-pdf in version 8.0.2 image So until we found a solution for this, the override doesn't work in all situations.

donovanclarke commented 1 month ago

The temporary override (pdfjs-dist -> 4.2.67) seems to fix the audit issues but we (@marvinjaworski ) are facing compatibility issues withreact-pdf in version 8.0.2 image So until we found a solution for this, the override doesn't work in all situations.

I mean you are bumping a major version of pdfjs. It is more than likely their will be some sort of breaking change.

You could try linting your project to find where the import error is happening and fix it there. You may run into the same issue even with this library updating the dependency.

a-str-o commented 1 month ago

"pdfjs-dist": "3.11.174", "react-pdf": "^0.0.10", this updated after i do npm i

nextjs app same problem

grueneerle commented 1 week ago

Fixed for us by switching to react-pdf 9.0.0 and adapting breaking changes.