Skip to content

Conversation

@asger-semmle
Copy link
Contributor

@asger-semmle asger-semmle commented Apr 5, 2019

Fixes a bug when the next use of x after x += e was not a value node.

I remember trying to use DataFlow::ssaDefinitionNode in the past and observing that it didn't work but was never able to figure out why. I've found the problem: StringConcatenationTaintStep extended DataFlow::ValueNode.

Copy link

@xiemaisi xiemaisi left a comment

Choose a reason for hiding this comment

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

A pleasantly simple fix! LGTM modulo evaluation.

@asger-semmle
Copy link
Contributor Author

Evaluation looks ok. There is a slow down on one project which I haven't investigated further, as I would expect it's just from the increased reach of taint tracking.

The new results seem like TPs except for the regexp ones, which are due to mismatched array indices when tracking taint through an array used as a tuple. I've opened ODASA-7893 to keep track of such FPs for when we want to fix the issue. I believe these particular FPs would go away after #1211 though.

@asger-semmle asger-semmle added the JS label Apr 8, 2019
@asger-semmle asger-semmle marked this pull request as ready for review April 8, 2019 10:06
@asger-semmle asger-semmle requested a review from a team as a code owner April 8, 2019 10:06
@semmle-qlci semmle-qlci merged commit f54366b into github:master Apr 8, 2019
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