Skip to content

Commit 87e0831

Browse files
committed
JS: Fix flow for nested destructurings
1 parent 6468721 commit 87e0831

File tree

3 files changed

+39
-1
lines changed

3 files changed

+39
-1
lines changed

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,23 @@ module DataFlow {
11431143
succ = TDestructuringPatternNode(def.getTarget())
11441144
)
11451145
or
1146+
// flow from the value read from a property pattern to the value being
1147+
// destructured in the child pattern. For example, for
1148+
//
1149+
// let { p: { q: x } } = obj
1150+
//
1151+
// add edge from the 'p:' pattern to '{ q:x }'.
1152+
exists(PropertyPattern pattern |
1153+
pred = TPropNode(pattern) and
1154+
succ = TDestructuringPatternNode(pattern.getValuePattern())
1155+
)
1156+
or
1157+
// Like the step above, but for array destructuring patterns.
1158+
exists(Expr elm |
1159+
pred = TElementPatternNode(_, elm) and
1160+
succ = TDestructuringPatternNode(elm)
1161+
)
1162+
or
11461163
// flow from 'this' parameter into 'this' expressions
11471164
exists(ThisExpr thiz |
11481165
pred = TThisNode(thiz.getBindingContainer()) and
@@ -1154,7 +1171,7 @@ module DataFlow {
11541171
* Holds if there is a step from `pred` to `succ` through a field accessed through `this` in a class.
11551172
*/
11561173
predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
1157-
exists (ClassNode cls, string prop |
1174+
exists(ClassNode cls, string prop |
11581175
pred = cls.getAReceiverNode().getAPropertyWrite(prop).getRhs() and
11591176
succ = cls.getAReceiverNode().getAPropertyRead(prop)
11601177
)

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
2323
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
2424
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
25+
| destruct.js:15:7:15:14 | source() | destruct.js:5:10:5:10 | z |
26+
| destruct.js:15:7:15:14 | source() | destruct.js:8:10:8:10 | w |
27+
| destruct.js:15:7:15:14 | source() | destruct.js:11:10:11:10 | q |
2528
| exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
2629
| exceptions.js:21:17:21:24 | source() | exceptions.js:23:10:23:10 | e |
2730
| exceptions.js:21:17:21:24 | source() | exceptions.js:24:10:24:21 | e.toString() |
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
function test() {
2+
3+
function f(obj) {
4+
let { x: { y: { z } } } = obj;
5+
sink(z); // NOT OK
6+
7+
let [[[w]]] = obj;
8+
sink(w); // NOT OK
9+
10+
let { x: [ { y: q } ] } = obj;
11+
sink(q); // NOT OK
12+
}
13+
14+
function g() {
15+
f(source());
16+
}
17+
18+
}

0 commit comments

Comments
 (0)