VadimDez / ng2-pdf-viewer

📄 PDF Viewer Component for Angular
https://vadimdez.github.io/ng2-pdf-viewer/
MIT License
1.28k stars 405 forks source link

PDF.js vulnerable to arbitrary JavaScript execution upon opening a malicious PDF #1088

Closed maximedupre closed 2 weeks ago

maximedupre commented 3 weeks ago
Bug Report or Feature Request (mark with an x)
- [ ] Regression (a behavior that used to work and stopped working in a new release)
- [ x ] Bug report -> please search issues before submitting
- [ ] Feature request
- [ ] Documentation issue or request

Received a Dependabot vulnerability alert:

PDF.js vulnerable to arbitrary JavaScript execution upon opening a malicious PDF Dependabot cannot update pdfjs-dist to a non-vulnerable version The latest possible version that can be installed is 2.16.105 because of the following conflicting dependencies:

ng2-pdf-viewer@10.0.0 requires pdfjs-dist@~2.16.105 No patched version available for pdfjs-dist The earliest fixed version is 4.2.67.

shamoon commented 3 weeks ago

This repo has been quiet for some time and pdfjs is pretty far behind now. Hoping the project does get revived but unclear. If moving to 4.x is not on the table, the workaround is to set the option isEvalSupported to false, thats our plan.

maximedupre commented 3 weeks ago

This repo has been quiet for some time and pdfjs is pretty far behind now. Hoping the project does get revived but unclear. If moving to 4.x is not on the table, the workaround is to set the option isEvalSupported to false, thats our plan.

That's a good workaround, although it will not resolve the Dependabot alert unless one chooses to dismiss the alert manually, which I suppose is a decent compromise, because I'm not sure this project will ever update 2 major versions to catch up with pdfjs 😅

shamoon commented 3 weeks ago

Yea, we ended up internalizing this package and upgrading pdfjs to 3.x but 4.x has so many breaking changes it'd be a lot of work to migrate. Hasn't been a meaningful update here for many months but you never know.

Bscout2011 commented 3 weeks ago

I forked and pushed a commit, but there are build issues with pdfjs-dist. The pdfjs github repo does not contain any code related to PromiseWithResolvers or WeakKey.

./node_modules/pdfjs-dist/build/pdf.mjs - Error: Module parse failed: The top-level-await experiment is not enabled (set experiments.topLevelAwait: true to enabled it)
File was processed with these loaders:
 * ./node_modules/@angular-devkit/build-angular/src/tools/babel/webpack-loader.js
 * ./node_modules/source-map-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
Error: The top-level-await experiment is not enabled (set experiments.topLevelAwait: true to enabled it)

./node_modules/pdfjs-dist/web/pdf_viewer.mjs - Error: Module parse failed: The top-level-await experiment is not enabled (set experiments.topLevelAwait: true to enabled it)
File was processed with these loaders:
 * ./node_modules/@angular-devkit/build-angular/src/tools/babel/webpack-loader.js
 * ./node_modules/source-map-loader/dist/cjs.js
You may need an additional loader to handle the result of these loaders.
Error: The top-level-await experiment is not enabled (set experiments.topLevelAwait: true to enabled it)

Error: node_modules/pdfjs-dist/types/src/display/api.d.ts:686:23 - error TS2304: Cannot find name 'PromiseWithResolvers'.
686     _readyCapability: PromiseWithResolvers<any>;
                          ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/src/display/api.d.ts:738:18 - error TS2304: Cannot find name 'PromiseWithResolvers'.
738     _capability: PromiseWithResolvers<any>;
                     ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/src/display/api.d.ts:1381:23 - error TS2304: Cannot find name 'PromiseWithResolvers'.
1381     _readyCapability: PromiseWithResolvers<any>;
                           ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/src/display/text_layer.d.ts:101:18 - error TS2304: Cannot find name 'PromiseWithResolvers'.
101     _capability: PromiseWithResolvers<any>;
                     ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/web/pdf_find_controller.d.ts:105:27 - error TS2304: Cannot find name 'PromiseWithResolvers'.
105     _firstPageCapability: PromiseWithResolvers<any> | undefined;
                              ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/web/pdf_viewer.d.ts:296:27 - error TS2304: Cannot find name 'PromiseWithResolvers'.
296     _firstPageCapability: PromiseWithResolvers<any> | undefined;
                              ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/web/pdf_viewer.d.ts:297:33 - error TS2304: Cannot find name 'PromiseWithResolvers'.
297     _onePageRenderedCapability: PromiseWithResolvers<any> | undefined;
                                    ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/web/pdf_viewer.d.ts:298:23 - error TS2304: Cannot find name 'PromiseWithResolvers'.
298     _pagesCapability: PromiseWithResolvers<any> | undefined;
                          ~~~~~~~~~~~~~~~~~~~~

Error: node_modules/pdfjs-dist/types/web/text_layer_builder.d.ts:36:32 - error TS2304: Cannot find name 'WeakKey'.
36     textDivProperties: WeakMap<WeakKey, any>;
                                  ~~~~~~~
× Failed to compile.

Would you all know how to resolve the build issue?

shamoon commented 3 weeks ago

Updating this project to pdfjs 4.x is going to require a lot more than just the changes in that commit to actually work, I think. Before 4 came out I opened a PR #1027 for 3.x but 4.x has lots of breaking changes. Unfortunately it seems this CVE patch is not going to be back-ported to 3.x (nor 2.x).

ftaffelt commented 2 weeks ago

@shamoon saw your fix to mitigate the isEvalSupported in the paperless-ngx project. I'm not really sure, but i think setting the global property to false: PDFJS['isEvalSupported'] = false https://github.com/paperless-ngx/paperless-ngx/blob/ac666df4ceea92a12fa648e885094a3a1b65a730/src-ui/src/app/components/common/pdf-viewer/pdf-viewer.component.ts#L38

does nothing, because there isn't a global property with this name. I think it once existed in a very, very old version of PDFJS.

Annother way to disable the evaluation of the JS code would be via the getDocument function, which supports configuration via the DocumentInitParameters object which in turn has a property isEvalSupported:

const params: any = {cMapUrl: this._cMapsUrl, cMapPacked: true, enableXfa: true, isEvalSupported:false } https://github.com/paperless-ngx/paperless-ngx/blob/ac666df4ceea92a12fa648e885094a3a1b65a730/src-ui/src/app/components/common/pdf-viewer/pdf-viewer.component.ts#L438-L442

shamoon commented 2 weeks ago

Thanks, docs are very sparse so I wasnt sure. Looks like it's in DocumentInitParameters https://mozilla.github.io/pdf.js/api/draft/module-pdfjsLib.html . Your suggestion would indeed default it to true but not force it to, I think just params.isEvalSupported = false will do it.

Thanks again.

maximedupre commented 2 weeks ago

@shamoon We still have to dismiss the vulnerability alert manually right?

shamoon commented 2 weeks ago

Yes, I believe so