Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
Added taint-steps for `Array.prototype.toSpliced`
21 changes: 16 additions & 5 deletions javascript/ql/lib/semmle/javascript/Arrays.qll
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,23 @@ module ArrayTaintTracking {
pred = call.getArgument(any(int i | i >= 2)) and
succ.(DataFlow::SourceNode).getAMethodCall("splice") = call
or
// `array.toSpliced(x, y, source())`: if `source()` is tainted, then so is the result of `toSpliced`, but not the original array.
call.(DataFlow::MethodCallNode).getMethodName() = "toSpliced" and
pred = call.getArgument(any(int i | i >= 2)) and
succ = call
or
// `array.splice(i, del, ...e)`: if `e` is tainted, then so is `array`.
pred = call.getASpreadArgument() and
succ.(DataFlow::SourceNode).getAMethodCall("splice") = call
or
// `array.toSpliced(i, del, ...e)`: if `e` is tainted, then so is the result of `toSpliced`, but not the original array.
pred = call.getASpreadArgument() and
call.(DataFlow::MethodCallNode).getMethodName() = "toSpliced" and
succ = call
or
// `e = array.pop()`, `e = array.shift()`, or similar: if `array` is tainted, then so is `e`.
call.(DataFlow::MethodCallNode).calls(pred, ["pop", "shift", "slice", "splice", "at"]) and
call.(DataFlow::MethodCallNode)
.calls(pred, ["pop", "shift", "slice", "splice", "at", "toSpliced"]) and
succ = call
or
// `e = Array.from(x)`: if `x` is tainted, then so is `e`.
Expand Down Expand Up @@ -283,7 +294,7 @@ private module ArrayDataFlow {
private class ArraySpliceStep extends PreCallGraphStep {
override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
exists(DataFlow::MethodCallNode call |
call.getMethodName() = "splice" and
call.getMethodName() = ["splice", "toSpliced"] and
prop = arrayElement() and
element = call.getArgument(any(int i | i >= 2)) and
call = obj.getAMethodCall()
Expand All @@ -297,7 +308,7 @@ private module ArrayDataFlow {
toProp = arrayElement() and
// `array.splice(i, del, ...arr)` variant
exists(DataFlow::MethodCallNode mcn |
mcn.getMethodName() = "splice" and
mcn.getMethodName() = ["splice", "toSpliced"] and
pred = mcn.getASpreadArgument() and
succ = mcn.getReceiver().getALocalSource()
)
Expand All @@ -320,12 +331,12 @@ private module ArrayDataFlow {
}

/**
* A step for modeling that elements from an array `arr` also appear in the result from calling `slice`/`splice`/`filter`.
* A step for modeling that elements from an array `arr` also appear in the result from calling `slice`/`splice`/`filter`/`toSpliced`.
*/
private class ArraySliceStep extends PreCallGraphStep {
override predicate loadStoreStep(DataFlow::Node pred, DataFlow::SourceNode succ, string prop) {
exists(DataFlow::MethodCallNode call |
call.getMethodName() = ["slice", "splice", "filter"] and
call.getMethodName() = ["slice", "splice", "filter", "toSpliced"] and
prop = arrayElement() and
pred = call.getReceiver() and
succ = call
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/lib/semmle/javascript/Regexp.qll
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,7 @@ private predicate isUsedAsNumber(DataFlow::LocalSourceNode value) {
or
exists(DataFlow::CallNode call |
call.getCalleeName() =
["substring", "substr", "slice", "splice", "charAt", "charCodeAt", "codePointAt"] and
["substring", "substr", "slice", "splice", "charAt", "charCodeAt", "codePointAt", "toSpliced"] and
value.flowsTo(call.getAnArgument())
)
}
Expand Down
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/Arrays/DataFlow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
Expand All @@ -22,3 +23,5 @@
| arrays.js:29:21:29:28 | "source" | arrays.js:50:8:50:17 | arr6.pop() |
| arrays.js:33:37:33:44 | "source" | arrays.js:35:8:35:25 | arr4_variant.pop() |
| arrays.js:53:4:53:11 | "source" | arrays.js:54:10:54:18 | ary.pop() |
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |
3 changes: 3 additions & 0 deletions javascript/ql/test/library-tests/Arrays/TaintFlow.expected
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
| arrays.js:2:16:2:23 | "source" | arrays.js:86:8:86:35 | arrayFi ... llback) |
| arrays.js:2:16:2:23 | "source" | arrays.js:90:10:90:10 | x |
| arrays.js:2:16:2:23 | "source" | arrays.js:93:8:93:17 | arr.at(-1) |
| arrays.js:2:16:2:23 | "source" | arrays.js:109:8:109:24 | arr8_spread.pop() |
| arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
| arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
| arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |
Expand All @@ -26,3 +27,5 @@
| arrays.js:53:4:53:11 | "source" | arrays.js:55:10:55:12 | ary |
| arrays.js:95:9:95:16 | "source" | arrays.js:95:8:95:34 | ["sourc ... ) => x) |
| arrays.js:96:9:96:16 | "source" | arrays.js:96:8:96:36 | ["sourc ... => !!x) |
| arrays.js:99:31:99:38 | "source" | arrays.js:100:8:100:17 | arr8.pop() |
| arrays.js:103:55:103:62 | "source" | arrays.js:105:8:105:25 | arr8_variant.pop() |
13 changes: 13 additions & 0 deletions javascript/ql/test/library-tests/Arrays/arrays.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,17 @@

sink(["source"].filter((x) => x)); // NOT OK
sink(["source"].filter((x) => !!x)); // NOT OK

var arr8 = [];
arr8 = arr8.toSpliced(0, 0, "source");
sink(arr8.pop()); // NOT OK

var arr8_variant = [];
arr8_variant = arr8_variant.toSpliced(0, 0, "safe", "source");
arr8_variant.pop();
sink(arr8_variant.pop()); // NOT OK

var arr8_spread = [];
arr8_spread = arr8_spread.toSpliced(0, 0, ...arr);
sink(arr8_spread.pop()); // NOT OK
});
Loading