feat: detect weak crypto algorithm #30
Conversation
Kawacrepe
left a comment
Kawacrepe
left a comment
There was a problem hiding this comment.
Lgtm! My only question is: shouldn't we add a valid test case ? where the cryptoHash algorithm used is something like sha256 etc..?
Very good idea ^^ I'll add it. |
|
@fraxken do you prefer to wait for the tracer feature? |
|
@tony-go No i guess this is ok to merge. For weak hash i have found this list
SHA1 is supported by Node.js (maybe we have to check for the other ones). |
src/probes/isWeakCrypto.js
Outdated
| import { warnings } from "../constants.js"; | ||
|
|
||
| // Constants | ||
| const weakAlgorithms = ["md5", "sha1", "ripemd160", "md4", "md2"]; |
There was a problem hiding this comment.
new Set(["md5", "sha1", "ripemd160", "md4", "md2"]) no ?
There was a problem hiding this comment.
Shame on me! this is the vim effect!
src/probes/isWeakCrypto.js
Outdated
| const isCryptoImported = analysis.dependencies.has("crypto"); | ||
|
|
||
| if ( | ||
| weakAlgorithms.includes(arg.value) && |
There was a problem hiding this comment.
weakAlgorithms.has(arg.value)With an ES6 Set
|
@all-contributors please add @tony-go for code,tests,docs |
|
I couldn't determine any contributions to add, did you specify any contributions? |
|
@all-contributors please add @tony for code, doc, test |
|
I've put up a pull request to add @tony! 🎉 |
Context
This PR resolves issue #25
Improvements 🚀
At the moment we detect:
md5md5sha1sha1ripemd160ripemd160md4md4md2md2Questions
import 'crypto'be required to trigger this warning?sha1, but TBH I don't know other ones 🙈)?