Skip to content

Commit 3f2befc

Browse files
committed
JS: Support spread arguments in array.splice
1 parent 269f8ca commit 3f2befc

File tree

3 files changed

+21
-2
lines changed

3 files changed

+21
-2
lines changed

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,14 @@ module ArrayTaintTracking {
7777
succ = call.getReceiver().getALocalSource() and
7878
call.getCalleeName() = ["push", "unshift"]
7979
or
80-
// `array.splice(i, del, ...items)`: if any item is tainted, then so is `array`.
80+
// `array.splice(i, del, e1, e2, ...)`: if any item is tainted, then so is `array`.
8181
pred = call.getArgument(any(int i | i >= 2)) and
8282
succ.(DataFlow::SourceNode).getAMethodCall("splice") = call
8383
or
84+
// `array.splice(i, del, ...e)`: if `e` is tainted, then so is `array`.
85+
pred = call.getASpreadArgument() and
86+
succ.(DataFlow::SourceNode).getAMethodCall("splice") = call
87+
or
8488
// `e = array.pop()`, `e = array.shift()`, or similar: if `array` is tainted, then so is `e`.
8589
call.(DataFlow::MethodCallNode).calls(pred, ["pop", "shift", "slice", "splice", "at"]) and
8690
succ = call
@@ -274,7 +278,7 @@ private module ArrayDataFlow {
274278

275279
/**
276280
* A step modeling that `splice` can insert elements into an array.
277-
* For example in `array.splice(i, del, ...items)`: if any item is tainted, then so is `array`
281+
* For example in `array.splice(i, del, e1, e2, ...)`: if any item is tainted, then so is `array`
278282
*/
279283
private class ArraySpliceStep extends PreCallGraphStep {
280284
override predicate storeStep(DataFlow::Node element, DataFlow::SourceNode obj, string prop) {
@@ -285,6 +289,19 @@ private module ArrayDataFlow {
285289
call = obj.getAMethodCall()
286290
)
287291
}
292+
293+
override predicate loadStoreStep(
294+
DataFlow::Node pred, DataFlow::SourceNode succ, string fromProp, string toProp
295+
) {
296+
fromProp = arrayLikeElement() and
297+
toProp = arrayElement() and
298+
// `array.splice(i, del, ...arr)` variant
299+
exists(DataFlow::MethodCallNode mcn |
300+
mcn.getMethodName() = "splice" and
301+
pred = mcn.getASpreadArgument() and
302+
succ = mcn.getReceiver().getALocalSource()
303+
)
304+
}
288305
}
289306

290307
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
| arrays.js:2:16:2:23 | "source" | arrays.js:15:27:15:27 | e |
44
| arrays.js:2:16:2:23 | "source" | arrays.js:16:23:16:23 | e |
55
| arrays.js:2:16:2:23 | "source" | arrays.js:20:8:20:16 | arr.pop() |
6+
| arrays.js:2:16:2:23 | "source" | arrays.js:39:8:39:24 | arr4_spread.pop() |
67
| arrays.js:2:16:2:23 | "source" | arrays.js:61:10:61:10 | x |
78
| arrays.js:2:16:2:23 | "source" | arrays.js:65:10:65:10 | x |
89
| arrays.js:2:16:2:23 | "source" | arrays.js:69:10:69:10 | x |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
| arrays.js:2:16:2:23 | "source" | arrays.js:15:27:15:27 | e |
44
| arrays.js:2:16:2:23 | "source" | arrays.js:16:23:16:23 | e |
55
| arrays.js:2:16:2:23 | "source" | arrays.js:20:8:20:16 | arr.pop() |
6+
| arrays.js:2:16:2:23 | "source" | arrays.js:39:8:39:24 | arr4_spread.pop() |
67
| arrays.js:2:16:2:23 | "source" | arrays.js:58:8:58:13 | arr[0] |
78
| arrays.js:2:16:2:23 | "source" | arrays.js:61:10:61:10 | x |
89
| arrays.js:2:16:2:23 | "source" | arrays.js:65:10:65:10 | x |

0 commit comments

Comments
 (0)