The Wayback Machine - https://web.archive.org/web/20211025233039/https://github.com/github/codeql/pull/6864
Skip to content
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

Jump to bottom

[Javascript] CWE-348: Client supplied ip used in security check #6864

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[Javascript] CWE-348: Client supplied ip used in security check #6864

wants to merge 4 commits into from

Conversation

@yabeow
Copy link

@yabeow yabeow commented Oct 13, 2021

Hi there,

This merge request ports these two similar queries to Javascript:

I also used local data flow to detect more sanitized cases in Javascript.
@yabeow
[Javascript] Add CWE-348 ClientSuppliedIpUsedInSecurityCheck
5cf15a3
@yabeow yabeow requested a review from as a code owner Oct 13, 2021
yabeow added 3 commits Oct 13, 2021
@yabeow
ClientSuppliedIpUsedInSecurityCheck: update examples
ede861c
@yabeow
ClientSuppliedIpUsedInSecurityCheck: update document & test cases
db50a71
@yabeow
ClientSuppliedIpUsedInSecurityCheck: eliminate some false positive cases
f297be7
@asgerf asgerf requested a review from esbena Oct 25, 2021
@esbena
Copy link
Contributor

@esbena esbena commented Oct 25, 2021

Thanks. This generally LGTM. I have a few suggestions for code quality improvements.
I think the source and sinks are a bit on the heuristic side, but I am happy to see them merged if they work well in practice.

(I trust that all of the learnings from the two other PRs have made it into this PR.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation JS
2 participants
@yabeow @esbena