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

Could you help remove the vulnerability introduced by package crypto-js? #1273

Closed
paimon0715 opened this issue Jul 7, 2021 · 11 comments
Closed

Comments

@paimon0715
Copy link

paimon0715 commented Jul 7, 2021

Hi,@blikblum @liborm85

Issue

1 vulnerabilities (high severity) is introduced in pdfkit:
Vulnerability SNYK-JS-CRYPTOJS-548472 (high severity) is detected in package crypto-js (versions:<3.2.1,>=3.3.0 <4.0.0):https://snyk.io/vuln/SNYK-JS-CRYPTOJS-548472
The above vulnerable package is referenced by pdfkit via:
[email protected][email protected]

Solution

Since [email protected].* is transitively referenced by 340 downstream projects (e.g., pdfmake 0.1.71 (latest version),
svg-to-pdfkit 0.1.8 (latest version), @compodoc/compodoc 1.1.11 (latest version), @amcharts/amcharts4 4.10.20 (latest version), admin-lte 3.1.0(latest version)),

[email protected].* is referenced by 76 downstream projects (e.g., @formbird/core 3.3.1 (latest version), @accordproject/ui-contract-editor 0.97.0 (latest version), ant-nodejs-kit 1.1.118 (latest version), byspectra-lib 1.4.483 (latest version), csf-pdfmake 0.1.7-0.2 (latest version)),

[email protected].* is referenced by 29 downstream projects (e.g., alphascript-api 3.0.2 (latest version), pdf-writer 1.1.2 (latest version), pdfmake2 1.0.1 (latest version), vtuzx-core 2.0.5 (latest version), @random-guys/blobber 0.2.4 (latest version)),

If pdfkit removes the vulnerable package from the above versions, then its fixed versions can help downstream users decrease their pain.It's kind of you to update packages in these versions.

Fixing suggestions

(1)In [email protected].*, you can kindly perform the following upgrades (not crossing their major versions):
crypto-js ^3.3.0 ➔ 3.2.1;

Note:
[email protected] has fixed the vulnerability SNYK-JS-CRYPTOJS-548472

(2)In [email protected].*, you can kindly perform the following upgrades (not crossing their major versions):
crypto-js ^3.1.9-1 ➔ 3.2.1;

Note:
[email protected] has fixed the vulnerability SNYK-JS-CRYPTOJS-548472

(3)In [email protected].*, you can kindly perform the following upgrades (not crossing their major versions):
crypto-js ^3.1.9-1 ➔ 3.2.1;

Note:
[email protected] has fixed the vulnerability SNYK-JS-CRYPTOJS-548472

Thank you for your attention to this issue!

Best regards,
Paimon

@paimon0715
Copy link
Author

Lots of downstream users transitively use [email protected].* and @0.10.* via unmaintained packages (the unmaintained packages cannot upgrade pdfkit from 0.11.* or 0.10.* to the latest version).So if you can kindly release fixed 0.11.* and 0.10.* versions, the downstream users can avoid introducing the vulnerablity.

@blikblum
Copy link
Member

blikblum commented Jul 8, 2021

@liborm85 updated to v3.3 but is also vulnerable. See context at #1095 (comment)

To get rid of vulnerability must update to v4 but it will make pdfkit incompatible with React Native.

@paimon0715
Copy link
Author

paimon0715 commented Jul 8, 2021

@blikblum The vulnerability affect the versions <3.2.1,>=3.3.0 <4.0.0 of package crypto-js https://snyk.io/vuln/SNYK-JS-CRYPTOJS-548472.
crypto-js@3.2.1 fixed the vulnerability SNYK-JS-CRYPTOJS-548472, which could be compatible with React Native.

@liborm85
Copy link
Collaborator

liborm85 commented Jul 8, 2021

Maybe crypto-js v4 in React Native with packagereact-native-get-random-values works? (source: brix/crypto-js#259 (comment))
But I can not try it.

@blikblum
Copy link
Member

blikblum commented Jul 9, 2021

Maybe crypto-js v4 in React Native with packagereact-native-get-random-values works? (source: brix/crypto-js#259 (comment))
But I can not try it.

I think is safe to upgrade to crypto-js v4

@paimon0715
Copy link
Author

@blikblum @liborm85 Thanks for your feedback. Of course it would be better, if crypto-js can be upgraded to v4.

@blikblum
Copy link
Member

blikblum commented Jul 10, 2021

I changed locally to crypto-js v4 but did not push a commit / PR because of the concern of file size in browser.

See this issue: brix/crypto-js#276

Other libraries that depends on crypto-js are having lot of issues: aws-amplify/amplify-js#7570

Better to stick with crypto-js 3.2.1 until the bundle size issue is resolved upstream

Version 3.2.1 also tries to import crypto module so have the bundle size issue

@liborm85
Copy link
Collaborator

Then the last ugly option is ignore crypto in browserify for standalone build (as is iconv-lite now). And I similarly ignore it in webpack in pdfmake.

@paimon0715
Copy link
Author

@liborm85 Thanks for your help.

@liborm85
Copy link
Collaborator

liborm85 commented Aug 6, 2021

Fix was released in 0.12.2.

@liborm85 liborm85 closed this as completed Aug 6, 2021
@paimon0715
Copy link
Author

@liborm85 Such a fix is the additional efforts that npm community brings to you.
The vulnerability patch in [email protected] can be automatically propagated into 340 projects.
Literally, it indeed benefits a huge amount of downstream users.
Thanks again.

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