Skip to content

Commit 37676f4

Browse files
committed
JS: Remove jump steps from IIFE steps
1 parent 7f2eae0 commit 37676f4

File tree

2 files changed

+22
-7
lines changed

2 files changed

+22
-7
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1217,14 +1217,29 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
12171217

12181218
predicate localMustFlowStep(Node node1, Node node2) { node1 = node2.getImmediatePredecessor() }
12191219

1220+
/**
1221+
* Holds if `node1 -> node2` should be removed as a jump step.
1222+
*
1223+
* Currently this is done as a workaround for the local steps generated from IIFEs.
1224+
*/
1225+
private predicate excludedJumpStep(Node node1, Node node2) {
1226+
exists(ImmediatelyInvokedFunctionExpr iife |
1227+
iife.argumentPassing(node2.asExpr(), node1.asExpr())
1228+
or
1229+
node1 = iife.getAReturnedExpr().flow() and
1230+
node2 = iife.getInvocation().flow()
1231+
)
1232+
}
1233+
12201234
/**
12211235
* Holds if data can flow from `node1` to `node2` through a non-local step
12221236
* that does not follow a call edge. For example, a step through a global
12231237
* variable.
12241238
*/
12251239
predicate jumpStep(Node node1, Node node2) {
12261240
valuePreservingStep(node1, node2) and
1227-
node1.getContainer() != node2.getContainer()
1241+
node1.getContainer() != node2.getContainer() and
1242+
not excludedJumpStep(node1, node2)
12281243
or
12291244
FlowSummaryPrivate::Steps::summaryJumpStep(node1.(FlowSummaryNode).getSummaryNode(),
12301245
node2.(FlowSummaryNode).getSummaryNode())

javascript/ql/test/library-tests/TripleDot/iife.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ function f1() {
44
return p; // argument to return
55
})(x);
66
}
7-
sink(inner(source("f1.1"))); // $ hasValueFlow=f1.1 SPURIOUS: hasValueFlow=f1.2
8-
sink(inner(source("f1.2"))); // $ hasValueFlow=f1.2 SPURIOUS: hasValueFlow=f1.1
7+
sink(inner(source("f1.1"))); // $ hasValueFlow=f1.1
8+
sink(inner(source("f1.2"))); // $ hasValueFlow=f1.2
99
}
1010

1111
function f2() {
@@ -16,8 +16,8 @@ function f2() {
1616
})(x);
1717
return y;
1818
}
19-
sink(inner(source("f2.1"))); // $ hasValueFlow=f2.1 SPURIOUS: hasValueFlow=f2.2
20-
sink(inner(source("f2.2"))); // $ hasValueFlow=f2.2 SPURIOUS: hasValueFlow=f2.1
19+
sink(inner(source("f2.1"))); // $ MISSING: hasValueFlow=f2.1
20+
sink(inner(source("f2.2"))); // $ MISSING: hasValueFlow=f2.2
2121
}
2222

2323
function f3() {
@@ -26,8 +26,8 @@ function f3() {
2626
return x; // captured variable to return
2727
})();
2828
}
29-
sink(inner(source("f3.1"))); // $ hasValueFlow=f3.1 SPURIOUS: hasValueFlow=f3.2
30-
sink(inner(source("f3.2"))); // $ hasValueFlow=f3.2 SPURIOUS: hasValueFlow=f3.1
29+
sink(inner(source("f3.1"))); // $ hasValueFlow=f3.1
30+
sink(inner(source("f3.2"))); // $ hasValueFlow=f3.2
3131
}
3232

3333
function f4() {

0 commit comments

Comments
 (0)