Skip to content

JS: add additional array-specific taint steps#192

Merged
semmle-qlci merged 4 commits into
masterfrom
unknown repository
Sep 14, 2018
Merged

JS: add additional array-specific taint steps#192
semmle-qlci merged 4 commits into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 13, 2018

This PR adds the array methods: .unshift, .pop, .shift, .slice, .splice, Array.from as taint steps. I also did a minor refactoring in TaintTracking.qll.

This change seems to have a minor overhead, wihin the margin of error. Evalution.

@ghost ghost self-requested a review as a code owner September 13, 2018 13:56
name = "shift" or
name = "slice" or
name = "splice" |
pred.(DataFlow::SourceNode).getAMethodCall(name) = call and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We don't normally backtrack the predecessor to its local source; only the successor needs to follow backward data flow in case of object mutation. Ordinary data flow will carry the taint from the source to the predecessor node anyway.

It might interfere with how flow through refinement nodes and guard sanitizers work (as it might bypass them), and create slight inconsistencies in path explanations. I'd prefer that we just do
pred = call.(DataFlow::MethodCall).getReceiver().

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch, amended as call.(DataFlow::MethodCallNode).calls(pred, name)

@asger-semmle
Copy link
Copy Markdown
Contributor

Otherwise LGTM

@ghost ghost added the JS label Sep 14, 2018
Copy link
Copy Markdown
Contributor

@asger-semmle asger-semmle 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 abbadf2 into github:master Sep 14, 2018
aibaars added a commit that referenced this pull request Oct 14, 2021
smowton pushed a commit to smowton/codeql that referenced this pull request Jan 17, 2022
Test case for generic inner type instantiation
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
Sync Main: More Misc Bugs (token related bugs)
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.

2 participants