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

Migrate from using crypto-js to the native browser crypto API #99

Open
secondl1ght opened this issue Oct 27, 2023 · 3 comments
Open

Migrate from using crypto-js to the native browser crypto API #99

secondl1ght opened this issue Oct 27, 2023 · 3 comments

Comments

@secondl1ght
Copy link
Contributor

Hello, I received a Critical Severity alert today from the crypto-js NPM package. I am sure you probably did as well, it stated that:

crypto-js PBKDF2 1,000 times weaker than specified in 1993 and 1.3M times weaker than current standard

I don't think lnc-web uses that algorithm but it is still concerning. So I checked out the repository for crypto-js and it seems that it is now deprecated. You can see the notice here: https://github.com/brix/crypto-js#discontinued

Active development of CryptoJS has been discontinued. This library is no longer maintained.

Nowadays, NodeJS and modern browsers have a native Crypto module. The latest version of CryptoJS already uses the native Crypto module for random number generation, since Math.random() is not crypto-safe. Further development of CryptoJS would result in it only being a wrapper of native Crypto. Therefore, development and maintenance has been discontinued, it is time to go for the native crypto module.

I think it would be a good idea for lnc-web to migrate away from this deprecated library ASAP. Even if the cryptography used by lnc-web from the crypto-js module is still considered secure, something as critical to security as this should be using the latest and greatest. The browser native crypto API is pretty good now so I don't think the migration should be too hard, but I haven't taken a full look at how everything works under the hood with lnc-web yet either.

https://developer.mozilla.org/en-US/docs/Web/API/Crypto

Please let me know your thoughts on this, thanks!

@secondl1ght
Copy link
Contributor Author

FYI this now shows up in NPM as well:

# npm audit report

crypto-js  <4.2.0
Severity: critical
crypto-js PBKDF2 1,000 times weaker than specified in 1993 and 1.3M times weaker than current standard - https://github.com/advisories/GHSA-xwcq-pm8m-c4vf
fix available via `npm audit fix --force`
Will install undefined@undefined, which is a breaking change
node_modules/crypto-js
  @lightninglabs/lnc-web  *
  Depends on vulnerable versions of crypto-js
  node_modules/@lightninglabs/lnc-web

@rolznz
Copy link

rolznz commented Dec 8, 2023

as a temporary workaround at least the dependency could be updated: https://github.com/brix/crypto-js/releases/tag/4.2.0

@secondl1ght
Copy link
Contributor Author

I can put in a PR to update the deps for this repo, unless you have some internal workflow you follow with tests whenever making updates?

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

2 participants