Skip to content

Commit ac1dd18

Browse files
asgerfasgerf
committed
JS: Remove taint step from array element to whole array
1 parent 5084d02 commit ac1dd18

5 files changed

Lines changed: 30 additions & 22 deletions

File tree

‎javascript/ql/lib/semmle/javascript/dataflow/TaintTracking.qll‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,14 @@ module TaintTracking {
260260
)
261261
}
262262

263+
private class HeapLegacyTaintStep extends LegacyTaintStep {
264+
override predicate heapStep(DataFlow::Node pred, DataFlow::Node succ) {
265+
// arrays with tainted elements are tainted (in old data flow)
266+
succ.(DataFlow::ArrayCreationNode).getAnElement() = pred and
267+
not any(PromiseAllCreation call).getArrayNode() = succ
268+
}
269+
}
270+
263271
/**
264272
* A taint propagating data flow edge through object or array elements and
265273
* promises.
@@ -274,10 +282,6 @@ module TaintTracking {
274282
// spreading a tainted value into an array literal gives a tainted array
275283
succ.(DataFlow::ArrayCreationNode).getASpreadArgument() = pred
276284
or
277-
// arrays with tainted elements and objects with tainted property names are tainted
278-
succ.(DataFlow::ArrayCreationNode).getAnElement() = pred and
279-
not any(PromiseAllCreation call).getArrayNode() = succ
280-
or
281285
// reading from a tainted object yields a tainted result
282286
succ.(DataFlow::PropRead).getBase() = pred and
283287
not (

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
legacyDataFlowDifference
2+
| array-mutation.js:31:33:31:40 | source() | array-mutation.js:32:8:32:8 | h | only flow with OLD data flow library |
3+
| array-mutation.js:35:36:35:43 | source() | array-mutation.js:36:8:36:8 | i | only flow with OLD data flow library |
4+
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:27:8:27:13 | arr[0] | only flow with OLD data flow library |
5+
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:33:8:33:13 | arr[0] | only flow with OLD data flow library |
6+
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:35:8:35:13 | arr[2] | only flow with OLD data flow library |
7+
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:36:8:36:13 | arr[3] | only flow with OLD data flow library |
8+
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:37:8:37:13 | arr[4] | only flow with OLD data flow library |
29
| bound-function.js:27:8:27:15 | source() | bound-function.js:30:10:30:10 | y | only flow with OLD data flow library |
10+
| call-apply.js:27:14:27:21 | source() | call-apply.js:24:8:24:11 | arg1 | only flow with NEW data flow library |
11+
| call-apply.js:27:14:27:21 | source() | call-apply.js:32:6:32:35 | foo1.ap ... e, ""]) | only flow with NEW data flow library |
12+
| call-apply.js:27:14:27:21 | source() | call-apply.js:34:6:34:29 | foo1_ap ... e, ""]) | only flow with NEW data flow library |
13+
| call-apply.js:27:14:27:21 | source() | call-apply.js:41:6:41:28 | foo1_ca ... ource]) | only flow with OLD data flow library |
14+
| call-apply.js:27:14:27:21 | source() | call-apply.js:59:10:59:21 | arguments[1] | only flow with OLD data flow library |
315
| call-apply.js:45:8:45:15 | source() | call-apply.js:55:6:55:13 | foo(obj) | only flow with NEW data flow library |
416
| callbacks.js:37:17:37:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |
517
| callbacks.js:37:17:37:24 | source() | callbacks.js:41:10:41:10 | x | only flow with NEW data flow library |
@@ -35,18 +47,11 @@ flow
3547
| array-mutation.js:19:18:19:25 | source() | array-mutation.js:20:8:20:8 | e |
3648
| array-mutation.js:23:13:23:20 | source() | array-mutation.js:24:8:24:8 | f |
3749
| array-mutation.js:27:16:27:23 | source() | array-mutation.js:28:8:28:8 | g |
38-
| array-mutation.js:31:33:31:40 | source() | array-mutation.js:32:8:32:8 | h |
39-
| array-mutation.js:35:36:35:43 | source() | array-mutation.js:36:8:36:8 | i |
4050
| array-mutation.js:39:17:39:24 | source() | array-mutation.js:40:8:40:8 | j |
4151
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:17:8:17:13 | arr[1] |
4252
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:22:8:22:13 | arr[6] |
43-
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:27:8:27:13 | arr[0] |
4453
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:28:8:28:13 | arr[1] |
45-
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:33:8:33:13 | arr[0] |
4654
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:34:8:34:13 | arr[1] |
47-
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:35:8:35:13 | arr[2] |
48-
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:36:8:36:13 | arr[3] |
49-
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:37:8:37:13 | arr[4] |
5055
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:38:8:38:13 | arr[5] |
5156
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:43:10:43:15 | arr[i] |
5257
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:55:10:55:15 | arr[i] |
@@ -69,11 +74,8 @@ flow
6974
| call-apply.js:27:14:27:21 | source() | call-apply.js:24:8:24:11 | arg1 |
7075
| call-apply.js:27:14:27:21 | source() | call-apply.js:29:6:29:32 | foo1.ca ... ce, "") |
7176
| call-apply.js:27:14:27:21 | source() | call-apply.js:32:6:32:35 | foo1.ap ... e, ""]) |
72-
| call-apply.js:27:14:27:21 | source() | call-apply.js:33:6:33:35 | foo2.ap ... e, ""]) |
7377
| call-apply.js:27:14:27:21 | source() | call-apply.js:34:6:34:29 | foo1_ap ... e, ""]) |
7478
| call-apply.js:27:14:27:21 | source() | call-apply.js:40:6:40:28 | foo1_ca ... e, ""]) |
75-
| call-apply.js:27:14:27:21 | source() | call-apply.js:41:6:41:28 | foo1_ca ... ource]) |
76-
| call-apply.js:27:14:27:21 | source() | call-apply.js:59:10:59:21 | arguments[1] |
7779
| call-apply.js:27:14:27:21 | source() | call-apply.js:62:10:62:21 | arguments[0] |
7880
| call-apply.js:45:8:45:15 | source() | call-apply.js:55:6:55:13 | foo(obj) |
7981
| call-apply.js:81:17:81:24 | source() | call-apply.js:78:8:78:11 | this |

‎javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
legacyDataFlowDifference
22
| arrays-init.js:2:16:2:23 | source() | arrays-init.js:38:8:38:13 | arr[5] | only flow with NEW data flow library |
33
| bound-function.js:27:8:27:15 | source() | bound-function.js:30:10:30:10 | y | only flow with OLD data flow library |
4+
| call-apply.js:27:14:27:21 | source() | call-apply.js:24:8:24:11 | arg1 | only flow with NEW data flow library |
5+
| call-apply.js:27:14:27:21 | source() | call-apply.js:32:6:32:35 | foo1.ap ... e, ""]) | only flow with NEW data flow library |
46
| call-apply.js:27:14:27:21 | source() | call-apply.js:34:6:34:29 | foo1_ap ... e, ""]) | only flow with NEW data flow library |
57
| call-apply.js:45:8:45:15 | source() | call-apply.js:55:6:55:13 | foo(obj) | only flow with NEW data flow library |
68
| callbacks.js:37:17:37:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |

‎javascript/ql/test/library-tests/TaintTracking/arrays-init.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,17 @@
2424

2525
console.log("=== access by index (init by [...]) ===");
2626
var arr = [str, source];
27-
sink(arr[0]); // OK [INCONSISTENCY]
27+
sink(arr[0]); // OK
2828
sink(arr[1]); // NOT OK
2929
sink(str); // OK
3030

3131
console.log("=== access by index (init by [...], array.lenght > 5) ===");
3232
var arr = [str, source, 'b', 'c', 'd', source];
33-
sink(arr[0]); // OK [INCONSISTENCY]
33+
sink(arr[0]); // OK
3434
sink(arr[1]); // NOT OK
35-
sink(arr[2]); // OK [INCONSISTENCY]
36-
sink(arr[3]); // OK [INCONSISTENCY]
37-
sink(arr[4]); // OK [INCONSISTENCY]
35+
sink(arr[2]); // OK
36+
sink(arr[3]); // OK
37+
sink(arr[4]); // OK
3838
sink(arr[5]); // NOT OK
3939

4040
console.log("=== access in for (init by [...]) ===");

‎javascript/ql/test/library-tests/TaintTracking/call-apply.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,15 @@ sink(foo1.call(null, source, "")); // NOT OK
3030
sink(foo2.call(null, source, "")); // OK
3131

3232
sink(foo1.apply(null, [source, ""])); // NOT OK
33-
sink(foo2.apply(null, [source, ""])); // OK [INCONSISTENCY]
33+
sink(foo2.apply(null, [source, ""])); // OK
3434
sink(foo1_apply([source, ""])); // NOT OK
3535

3636
foo1_apply_sink([source, ""]); // This works, because we don't need a return after a call (the sink is inside the called function).
3737

3838
sink(foo1_apply.apply(["", source])); // OK
3939

4040
sink(foo1_call([source, ""])); // NOT OK
41-
sink(foo1_call(["", source])); // OK [INCONSISTENCY]
41+
sink(foo1_call(["", source])); // OK
4242

4343

4444
var obj = {
@@ -56,7 +56,7 @@ sink(foo(obj)); // NOT OK
5656

5757
function argumentsObject() {
5858
function sinkArguments1() {
59-
sink(arguments[1]); // OK [INCONSISTENCY]
59+
sink(arguments[1]); // OK
6060
}
6161
function sinkArguments0() {
6262
sink(arguments[0]); // NOT OK

0 commit comments

Comments
 (0)