Skip to content

Commit 66d6bda

Browse files
authored
Merge pull request #18044 from asgerf/js/shared-dataflow-bump
JS: Merge 'main' and implement 'speculativeTaintStep'
2 parents 82d61e4 + 805fd0b commit 66d6bda

File tree

12 files changed

+423
-143
lines changed

12 files changed

+423
-143
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll

+19
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,22 @@ predicate defaultImplicitTaintRead(DataFlow::Node node, ContentSet c) {
104104
// Optional steps are added through isAdditionalFlowStep but we don't want the implicit reads
105105
not optionalStep(node, _, _)
106106
}
107+
108+
private predicate isArgumentToResolvedCall(DataFlow::Node arg) {
109+
exists(DataFlowCall c |
110+
exists(viableCallable(c)) and
111+
isArgumentNode(arg, c, _)
112+
)
113+
}
114+
115+
predicate speculativeTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
116+
exists(DataFlow::CallNode call |
117+
node1 = call.getAnArgument() and
118+
node2 = call and
119+
// A given node can appear as argument in more than one call. For example `x` in `fn.call(x)` is
120+
// is argument 0 of the `fn.call` call, but also the receiver of a reflective call to `fn`.
121+
// It is thus not enough to check if `call` has a known target; we nede to ensure that none of the calls
122+
// involving `node1` have a known target.
123+
not isArgumentToResolvedCall(node1)
124+
)
125+
}

javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionQuery.qll

-4
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ module IndirectCommandInjectionConfig implements DataFlow::ConfigSig {
2626
predicate isSink(DataFlow::Node sink) { isSinkWithHighlight(sink, _) }
2727

2828
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
29-
30-
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
31-
argsParseStep(pred, succ)
32-
}
3329
}
3430

3531
/**

javascript/ql/test/experimental/Security/CWE-918/SSRF.expected

+4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ edges
1212
| check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted | provenance | |
1313
| check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted | provenance | |
1414
| check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted | provenance | |
15+
| check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted | provenance | |
1516
| check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted | provenance | |
1617
| check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted | provenance | |
1718
| check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted | provenance | |
@@ -47,6 +48,8 @@ nodes
4748
| check-regex.js:34:25:34:42 | req.params.tainted | semmle.label | req.params.tainted |
4849
| check-regex.js:41:13:41:43 | "test.c ... tainted | semmle.label | "test.c ... tainted |
4950
| check-regex.js:41:27:41:43 | req.query.tainted | semmle.label | req.query.tainted |
51+
| check-regex.js:61:15:61:42 | baseURL ... tainted | semmle.label | baseURL ... tainted |
52+
| check-regex.js:61:25:61:42 | req.params.tainted | semmle.label | req.params.tainted |
5053
| check-validator.js:15:15:15:45 | "test.c ... tainted | semmle.label | "test.c ... tainted |
5154
| check-validator.js:15:29:15:45 | req.query.tainted | semmle.label | req.query.tainted |
5255
| check-validator.js:27:15:27:45 | "test.c ... tainted | semmle.label | "test.c ... tainted |
@@ -76,6 +79,7 @@ subpaths
7679
| check-regex.js:31:15:31:45 | "test.c ... tainted | check-regex.js:31:29:31:45 | req.query.tainted | check-regex.js:31:15:31:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
7780
| check-regex.js:34:15:34:42 | baseURL ... tainted | check-regex.js:34:25:34:42 | req.params.tainted | check-regex.js:34:15:34:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |
7881
| check-regex.js:41:13:41:43 | "test.c ... tainted | check-regex.js:41:27:41:43 | req.query.tainted | check-regex.js:41:13:41:43 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
82+
| check-regex.js:61:15:61:42 | baseURL ... tainted | check-regex.js:61:25:61:42 | req.params.tainted | check-regex.js:61:15:61:42 | baseURL ... tainted | The URL of this request depends on a user-provided value. |
7983
| check-validator.js:15:15:15:45 | "test.c ... tainted | check-validator.js:15:29:15:45 | req.query.tainted | check-validator.js:15:15:15:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
8084
| check-validator.js:27:15:27:45 | "test.c ... tainted | check-validator.js:27:29:27:45 | req.query.tainted | check-validator.js:27:15:27:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |
8185
| check-validator.js:50:15:50:45 | "test.c ... tainted | check-validator.js:50:29:50:45 | req.query.tainted | check-validator.js:50:15:50:45 | "test.c ... tainted | The URL of this request depends on a user-provided value. |

javascript/ql/test/library-tests/Arrays/DataFlow.expected

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ flow
1717
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
1818
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
1919
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
20+
| arrays.js:2:16:2:23 | "source" | arrays.js:110:8:110:24 | arr8_spread.pop() |
2021
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
2122
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
2223
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
@@ -28,3 +29,5 @@ flow
2829
| arrays.js:53:4:53:11 | "source" | arrays.js:54:10:54:18 | ary.pop() |
2930
| arrays.js:96:9:96:16 | "source" | arrays.js:96:8:96:40 | ["sourc ... ).pop() |
3031
| arrays.js:97:9:97:16 | "source" | arrays.js:97:8:97:42 | ["sourc ... ).pop() |
32+
| arrays.js:100:31:100:38 | "source" | arrays.js:101:8:101:17 | arr8.pop() |
33+
| arrays.js:104:55:104:62 | "source" | arrays.js:106:8:106:25 | arr8_variant.pop() |

javascript/ql/test/library-tests/Arrays/TaintFlow.expected

+3
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ flow
1616
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
1717
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
1818
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
19+
| arrays.js:2:16:2:23 | "source" | arrays.js:110:8:110:24 | arr8_spread.pop() |
1920
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
2021
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
2122
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
@@ -29,3 +30,5 @@ flow
2930
| arrays.js:95:9:95:16 | "source" | arrays.js:95:8:95:17 | ["source"] |
3031
| arrays.js:96:9:96:16 | "source" | arrays.js:96:8:96:40 | ["sourc ... ).pop() |
3132
| arrays.js:97:9:97:16 | "source" | arrays.js:97:8:97:42 | ["sourc ... ).pop() |
33+
| arrays.js:100:31:100:38 | "source" | arrays.js:101:8:101:17 | arr8.pop() |
34+
| arrays.js:104:55:104:62 | "source" | arrays.js:106:8:106:25 | arr8_variant.pop() |

0 commit comments

Comments
 (0)