Skip to content

Commit df4b596

Browse files
committed
Added toSpliced as part ArraySliceStep and ArraySpliceStep, fixed tests from 2d9bc43
1 parent 2d9bc43 commit df4b596

File tree

5 files changed

+350
-115
lines changed

5 files changed

+350
-115
lines changed

javascript/ql/lib/semmle/javascript/Arrays.qll

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ module ArrayTaintTracking {
8686
succ.(DataFlow::SourceNode).getAMethodCall("splice") = call
8787
or
8888
// `e = array.pop()`, `e = array.shift()`, or similar: if `array` is tainted, then so is `e`.
89-
call.(DataFlow::MethodCallNode).calls(pred, ["pop", "shift", "slice", "splice", "at", "toSpliced"]) and
89+
call.(DataFlow::MethodCallNode)
90+
.calls(pred, ["pop", "shift", "slice", "splice", "at", "toSpliced"]) and
9091
succ = call
9192
or
9293
// `e = Array.from(x)`: if `x` is tainted, then so is `e`.
@@ -283,7 +284,7 @@ private module ArrayDataFlow {
283284
private class ArraySpliceStep extends PreCallGraphStep {
284285
override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
285286
exists(DataFlow::MethodCallNode call |
286-
call.getMethodName() = "splice" and
287+
call.getMethodName() = ["splice", "toSpliced"] and
287288
prop = arrayElement() and
288289
element = call.getArgument(any(int i | i >= 2)) and
289290
call = obj.getAMethodCall()
@@ -297,7 +298,7 @@ private module ArrayDataFlow {
297298
toProp = arrayElement() and
298299
// `array.splice(i, del, ...arr)` variant
299300
exists(DataFlow::MethodCallNode mcn |
300-
mcn.getMethodName() = "splice" and
301+
mcn.getMethodName() = ["splice", "toSpliced"] and
301302
pred = mcn.getASpreadArgument() and
302303
succ = mcn.getReceiver().getALocalSource()
303304
)
@@ -320,12 +321,12 @@ private module ArrayDataFlow {
320321
}
321322

322323
/**
323-
* A step for modeling that elements from an array `arr` also appear in the result from calling `slice`/`splice`/`filter`.
324+
* A step for modeling that elements from an array `arr` also appear in the result from calling `slice`/`splice`/`filter`/`toSpliced`.
324325
*/
325326
private class ArraySliceStep extends PreCallGraphStep {
326327
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
327328
exists(DataFlow::MethodCallNode call |
328-
call.getMethodName() = ["slice", "splice", "filter"] and
329+
call.getMethodName() = ["slice", "splice", "filter", "toSpliced"] and
329330
prop = arrayElement() and
330331
pred = call.getReceiver() and
331332
succ = call

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
1414
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
1515
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
16+
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
1617
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
1718
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
1819
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
@@ -22,3 +23,5 @@
2223
| arrays.js:29:21:29:28 | "source" | arrays.js:50:8:50:17 | arr6.pop() |
2324
| arrays.js:33:37:33:44 | "source" | arrays.js:35:8:35:25 | arr4_variant.pop() |
2425
| arrays.js:53:4:53:11 | "source" | arrays.js:54:10:54:18 | ary.pop() |
26+
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
27+
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
1515
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
1616
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
17+
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
1718
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
1819
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
1920
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
@@ -26,3 +27,5 @@
2627
| arrays.js:53:4:53:11 | "source" | arrays.js:55:10:55:12 | ary |
2728
| arrays.js:95:9:95:16 | "source" | arrays.js:95:8:95:34 | ["sourc ... ) => x) |
2829
| arrays.js:96:9:96:16 | "source" | arrays.js:96:8:96:36 | ["sourc ... => !!x) |
30+
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
31+
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |

javascript/ql/test/library-tests/Arrays/arrays.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,14 +97,14 @@
9797

9898
var arr8 = [];
9999
arr8 = arr8.toSpliced(0, 0, "source");
100-
sink(arr8.pop()); // NOT OK -- Should be considered tainted, but it is not
100+
sink(arr8.pop()); // NOT OK
101101

102102
var arr8_variant = [];
103103
arr8_variant = arr8_variant.toSpliced(0, 0, "safe", "source");
104104
arr8_variant.pop();
105-
sink(arr8_variant.pop()); // NOT OK -- Should be considered tainted, but it is not
105+
sink(arr8_variant.pop()); // NOT OK
106106

107107
var arr8_spread = [];
108108
arr8_spread = arr8_spread.toSpliced(0, 0, ...arr);
109-
sink(arr8_spread.pop()); // NOT OK -- Should be considered tainted, but it is not
109+
sink(arr8_spread.pop()); // NOT OK
110110
});

0 commit comments

Comments
 (0)