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

[yabeow]

Hi there,

This merge request ports these two similar queries to Javascript:

• [Java] CWE-348: Using a client-supplied IP address in a security check #5631
• [Python] CWE-348: Client supplied ip used in security check #6214

I also used local data flow to detect more sanitized cases in Javascript.

[esbena]

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.)

[esbena]

getAChildExpr should generally not be used, but rather something like getIndex as seen above. What is being expressed in this final conjunct?

[yabeow]

I used getAChildExpr to get the variable being indexed. I think this is the correct way to do this, do you have any other suggestions?

This is the AST tree of temp[temp.length - 1];

[esbena]

I see now, thanks for clarifying. We should just stick to the DataFlow library then. Something like the following should do the trick:

    exists(DataFlow::MethodCallNode split, DataFlow::PropRead read |
       split = node and
      split.getMethodName() = "split" and // n.split(...)
       read = split.getAPropertyRead() // <n.split(...)>[...]
      not read.getPropertyNameExpr().getIntValue() = 0 and

[yabeow]

Hi @esbena,

Sorry for the late reply, I have modified the isSanitizer part and it should look good now. Thanks for your awesome suggestions. Do you have a moment to take a look at it again?

[yabeow]

Friendly ping! @esbena

[yabeow]

@esbena Hey there!

[esbena]

Sorry about the delay. I'll take a look now.

[esbena]

Thank you, there's one http/HTTP casing mistake left to fix. I have started a final performance check of the query, I'll approve once it looks good.

[esbena]

Have you run this query with vscode and viewed the results?
Queries with @kind path-problem must import PathGraph to output path explanations...

Example:

codeql/javascript/ql/src/Security/CWE-079/Xss.ql

Line 17 in 017183e

import DataFlow::PathGraph

[esbena]

Draft-stating the PR due to inactivity, but feel free to undraft when you have implemented the requested changes.