Skip to content

Commit 4c13e6b

Browse files
author
Esben Sparre Andreasen
committed
JS: add additional array-specific taint steps
1 parent 763da72 commit 4c13e6b

File tree

3 files changed

+60
-2
lines changed

3 files changed

+60
-2
lines changed

javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,28 @@ module TaintTracking {
239239
succ = call
240240
)
241241
or
242-
// `array.push(e)`: if `e` is tainted, then so is `array`
243-
succ.(DataFlow::SourceNode).getAMethodCall("push") = call
242+
// `array.push(e)`, `array.unshift(e)`: if `e` is tainted, then so is `array`.
243+
exists (string name |
244+
name = "push" or
245+
name = "unshift" |
246+
pred = call.getAnArgument() and
247+
succ.(DataFlow::SourceNode).getAMethodCall(name) = call
248+
)
249+
or
250+
// `e = array.pop()`, `e = array.shift()`, or similar: if `array` is tainted, then so is `e`.
251+
exists (string name |
252+
name = "pop" or
253+
name = "shift" or
254+
name = "slice" or
255+
name = "splice" |
256+
call.(DataFlow::MethodCallNode).calls(pred, name) and
257+
succ = call
258+
)
259+
or
260+
// `e = Array.from(x)`: if `x` is tainted, then so is `e`.
261+
call = DataFlow::globalVarRef("Array").getAPropertyRead("from").getACall() and
262+
pred = call.getAnArgument() and
263+
succ = call
244264
}
245265

246266
}

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,15 @@
33
| tst.js:2:13:2:20 | source() | tst.js:14:10:14:17 | x.sort() |
44
| tst.js:2:13:2:20 | source() | tst.js:17:10:17:10 | a |
55
| tst.js:2:13:2:20 | source() | tst.js:19:10:19:10 | a |
6+
| tst.js:2:13:2:20 | source() | tst.js:23:10:23:10 | b |
7+
| tst.js:2:13:2:20 | source() | tst.js:25:10:25:16 | x.pop() |
8+
| tst.js:2:13:2:20 | source() | tst.js:26:10:26:18 | x.shift() |
9+
| tst.js:2:13:2:20 | source() | tst.js:27:10:27:18 | x.slice() |
10+
| tst.js:2:13:2:20 | source() | tst.js:28:10:28:19 | x.splice() |
11+
| tst.js:2:13:2:20 | source() | tst.js:30:10:30:22 | Array.from(x) |
12+
| tst.js:2:13:2:20 | source() | tst.js:33:14:33:16 | elt |
13+
| tst.js:2:13:2:20 | source() | tst.js:35:14:35:16 | ary |
14+
| tst.js:2:13:2:20 | source() | tst.js:39:14:39:16 | elt |
15+
| tst.js:2:13:2:20 | source() | tst.js:41:14:41:16 | ary |
16+
| tst.js:2:13:2:20 | source() | tst.js:44:10:44:30 | innocen ... ) => x) |
17+
| tst.js:2:13:2:20 | source() | tst.js:45:10:45:24 | x.map(x2 => x2) |

javascript/ql/test/library-tests/TaintTracking/tst.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,30 @@ function test() {
1818
a.push(x);
1919
sink(a); // NOT OK
2020

21+
var b = [];
22+
b.unshift(x);
23+
sink(b); // NOT OK
24+
25+
sink(x.pop()); // NOT OK
26+
sink(x.shift()); // NOT OK
27+
sink(x.slice()); // NOT OK
28+
sink(x.splice()); // NOT OK
29+
30+
sink(Array.from(x)); // NOT OK
31+
32+
x.map((elt, i, ary) => {
33+
sink(elt); // NOT OK
34+
sink(i); // OK
35+
sink(ary); // NOT OK
36+
});
37+
38+
x.forEach((elt, i, ary) => {
39+
sink(elt); // NOT OK
40+
sink(i); // OK
41+
sink(ary); // NOT OK
42+
});
43+
44+
sink(innocent.map(() => x)); // NOT OK
45+
sink(x.map(x2 => x2)); // NOT OK
46+
2147
}

0 commit comments

Comments
 (0)