Skip to content

JS: support push and sort taint steps for arrays #117

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

Merged
merged 2 commits into from Sep 3, 2018
Merged

JS: support push and sort taint steps for arrays #117

merged 2 commits into from Sep 3, 2018

Conversation

ghost
Copy link

@ghost ghost commented Aug 30, 2018

This PR adds taint steps for Array.prototype.sort and Array.prototype.push calls.

Tagging for 1.18, but marking as WIP, as performance is still being evaluated.

The push step is flow-insensitive, I am curious about how that will work play out in practice.

@ghost ghost added the JS label Aug 30, 2018
@ghost ghost added this to the 1.18 milestone Aug 30, 2018
@ghost ghost self-requested a review as a code owner August 30, 2018 07:27
@ghost ghost added the WIP This is a work-in-progress, do not merge yet! label Aug 30, 2018
@ghost
Copy link
Author

ghost commented Aug 31, 2018

No new results or performance changes for big-apps.slugs internal link.
Continuing on default.slugs to check for false positives.

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.

LGTM

@xiemaisi xiemaisi changed the base branch from master to rc/1.18 August 31, 2018 19:40
@ghost
Copy link
Author

ghost commented Sep 3, 2018

Two FPs for js/remote-property-injection on default.slugs, but this PR is not the root cause of that (internal link).

Removing WIP label.

@ghost ghost removed the WIP This is a work-in-progress, do not merge yet! label Sep 3, 2018
@xiemaisi
Copy link

xiemaisi commented Sep 3, 2018

Intriguing; are the FPs due to deficiencies of the query?

@ghost
Copy link
Author

ghost commented Sep 3, 2018

See analysis in the gist

@xiemaisi
Copy link

xiemaisi commented Sep 3, 2018

OK, merging.

@xiemaisi xiemaisi merged commit 759d986 into github:rc/1.18 Sep 3, 2018
smowton pushed a commit to smowton/codeql that referenced this pull request Dec 6, 2021
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
erik-krogh pushed a commit to erik-krogh/ql that referenced this pull request Dec 15, 2021
…-generator

Upgrade the extractor generator
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Fixing a false positive in cs/insecure-sql-connection
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.

1 participant