Skip to content

Commit

Permalink
Merge pull request #17977 from Napalys/napalys/toSpliced-support
Browse files Browse the repository at this point in the history
JS: Added support for Array.prototype.toSpliced() ES2023 feature
  • Loading branch information
Napalys authored Nov 18, 2024
2 parents e081b9a + 631a377 commit 63bc1ef
Show file tree
Hide file tree
Showing 13 changed files with 440 additions and 117 deletions.
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

0 comments on commit 63bc1ef

Please sign in to comment.