-
Notifications
You must be signed in to change notification settings - Fork 45
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
[SINT-1547] Add NPM confusion analyzer #283
[SINT-1547] Add NPM confusion analyzer #283
Conversation
…re generated from same pyproject.toml and poetry.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a short description to the NPM metadata heuristics would be a good addition. Otherwise, LGTM!
@@ -319,6 +319,7 @@ | |||
} | |||
] | |||
}, | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! We tested this new check with @jamessteel123 on the https://www.npmjs.com/package/darcyclarke-manifest-pkg NPM package, a proof of concept for this attack.
This adds an analyzer which detects NPM confusion events. Specifically it checks when the version's manifest in NPM does not match the content in the package.json in a manner that can change the behavior of the code, or mislead the consumer of the package (report bugs to the wrong place, link to incorrect documentation).