Sitelet https://web.archive.org/web/20260601135430/https://github.com/github/codeql/pull/4342
Skip to content

JS: type-track objects where the "$where" property has been written#4342

Merged
codeql-ci merged 1 commit into
github:mainfrom
erik-krogh:track-where-prop
Sep 29, 2020
Merged

JS: type-track objects where the "$where" property has been written#4342
codeql-ci merged 1 commit into
github:mainfrom
erik-krogh:track-where-prop

Conversation

@erik-krogh
Copy link
Copy Markdown
Contributor

@erik-krogh erik-krogh commented Sep 24, 2020 •

Gets a TP for js/code-injection on this synthetic benchmark.

An alternative would be to add a $where step as an additional taint step (with flow-labels).
But that requires a larger refactoring of the $where sink, and could end up costing performance.

With this solution there can be a disconnect between a query-call and the alert location.
But I don't see a good solution to this.

@github-actions github-actions Bot added the JS label Sep 24, 2020
@erik-krogh erik-krogh marked this pull request as ready for review September 29, 2020 08:07
@erik-krogh erik-krogh requested a review from a team as a code owner September 29, 2020 08:07
Copy link
Copy Markdown
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

LGTM

I've confirmed that API graphs would have given us this "for free" using getMember("$where").getRhs() on the relevant node 🎉 . But that's blocked on porting the NoSQL library to API graphs, so I think it's ok to land this PR as it is.

@asgerf
Copy link
Copy Markdown
Contributor

asgerf commented Sep 29, 2020

But if you want to port the NoSQL library to API graphs that would also be great 😉

@codeql-ci codeql-ci merged commit 11f39a9 into github:main Sep 29, 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