Skip to content

JS: Flag deep assignments in prototype pollution query#2913

Merged
semmle-qlci merged 1 commit intogithub:masterfrom
asger-semmle:js/prototype-pollution-path
Mar 3, 2020
Merged

JS: Flag deep assignments in prototype pollution query#2913
semmle-qlci merged 1 commit intogithub:masterfrom
asger-semmle:js/prototype-pollution-path

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Feb 25, 2020

Expands the scope of the prototype pollution utility query to recognize "deep assignment" functions by treating x.split(".") as a source of polluted property names.

Evaluation looks reasonable. The new results aren't terribly interesting but looks legit. The only noticable slow down is in closure-library where there is also a new result, so the cost seems proportional.

@asgerf asgerf added the JS label Feb 25, 2020
@asgerf asgerf requested a review from a team as a code owner February 25, 2020 12:52
@asgerf asgerf force-pushed the js/prototype-pollution-path branch from a43d754 to 145697b Compare February 25, 2020 12:56
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good overall.

Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ready to approve after these.

@asgerf asgerf force-pushed the js/prototype-pollution-path branch from fa8a668 to 91843a8 Compare February 27, 2020 10:52
@asgerf
Copy link
Contributor Author

asgerf commented Feb 27, 2020

Rebased to resolve change note conflict

erik-krogh
erik-krogh previously approved these changes Feb 27, 2020
Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming the tests are green.

@asgerf
Copy link
Contributor Author

asgerf commented Feb 27, 2020

Squashed and rebased to resolve autoformat conflicts

Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@semmle-qlci semmle-qlci merged commit 7f3f629 into github:master Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants