Skip to content

Commit b5b89c0

Browse files
author
Max Schaefer
committed
JavaScript: Track flow into method receivers.
1 parent b3e8103 commit b5b89c0

File tree

7 files changed

+35
-11
lines changed

7 files changed

+35
-11
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ private predicate summarizedHigherOrderCall(
795795
) {
796796
exists(
797797
Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
798-
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary
798+
DataFlow::Node innerArg, DataFlow::SourceNode cbParm, PathSummary oldSummary
799799
|
800800
reachableFromInput(f, outer, arg, innerArg, cfg, oldSummary) and
801801
argumentPassing(outer, cb, f, cbParm) and

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ private module NodeTracking {
215215
basicLoadStep(pred, succ, prop) and
216216
summary = PathSummary::level()
217217
or
218-
exists (Function f, DataFlow::ParameterNode parm |
218+
exists (Function f, DataFlow::SourceNode parm |
219219
argumentPassing(succ, pred, f, parm) and
220220
reachesReturn(f, parm.getAPropertyRead(prop), summary)
221221
)
@@ -263,7 +263,7 @@ private module NodeTracking {
263263
) {
264264
exists(
265265
Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
266-
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary
266+
DataFlow::Node innerArg, DataFlow::SourceNode cbParm, PathSummary oldSummary
267267
|
268268
reachableFromInput(f, outer, arg, innerArg, oldSummary) and
269269
argumentPassing(outer, cb, f, cbParm) and

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,27 @@ private module CachedSteps {
125125
*/
126126
cached
127127
predicate argumentPassing(
128-
DataFlow::InvokeNode invk, DataFlow::ValueNode arg, Function f, DataFlow::ParameterNode parm
128+
DataFlow::InvokeNode invk, DataFlow::ValueNode arg, Function f, DataFlow::SourceNode parm
129129
) {
130130
calls(invk, f) and
131-
exists(int i |
132-
f.getParameter(i) = parm.getParameter() and
133-
not parm.isRestParameter() and
134-
arg = invk.getArgument(i)
131+
(
132+
exists(int i, Parameter p |
133+
f.getParameter(i) = p and
134+
not p.isRestParameter() and
135+
arg = invk.getArgument(i) and
136+
parm = DataFlow::parameterNode(p)
137+
)
138+
or
139+
arg = invk.(DataFlow::MethodCallNode).getReceiver() and
140+
parm = DataFlow::thisNode(f)
135141
)
136142
or
137-
exists(DataFlow::Node callback, int i |
143+
exists(DataFlow::Node callback, int i, Parameter p |
138144
invk.(DataFlow::AdditionalPartialInvokeNode).isPartialArgument(callback, arg, i) and
139145
partiallyCalls(invk, callback, f) and
140-
parm.getParameter() = f.getParameter(i) and
141-
not parm.isRestParameter()
146+
f.getParameter(i) = p and
147+
not p.isRestParameter() and
148+
parm = DataFlow::parameterNode(p)
142149
)
143150
}
144151

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
| tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
3939
| tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
4040
| tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
41+
| tst6.mjs:12:14:12:21 | "source" | tst6.mjs:14:12:14:16 | a.m() |
4142
| tst.js:2:17:2:22 | "src1" | tst.js:39:17:39:17 | x |
4243
| tst.js:2:17:2:22 | "src1" | tst.js:41:19:41:19 | x |
4344
| tst.js:2:17:2:22 | "src1" | tst.js:45:17:45:17 | x |

javascript/ql/test/library-tests/InterProceduralFlow/GermanFlow.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
| tst2.js:3:17:3:26 | "tainted2" | tst2.js:11:15:11:24 | g(source2) |
4040
| tst2.js:6:24:6:37 | "also tainted" | tst2.js:10:15:10:24 | g(source1) |
4141
| tst2.js:6:24:6:37 | "also tainted" | tst2.js:11:15:11:24 | g(source2) |
42+
| tst6.mjs:12:14:12:21 | "source" | tst6.mjs:14:12:14:16 | a.m() |
4243
| tst.js:2:17:2:22 | "src1" | tst.js:39:17:39:17 | x |
4344
| tst.js:2:17:2:22 | "src1" | tst.js:41:19:41:19 | x |
4445
| tst.js:2:17:2:22 | "src1" | tst.js:45:17:45:17 | x |

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
| tst4.js:2:16:2:24 | "tainted" | tst4.js:15:15:15:31 | id(still_tainted) |
4949
| tst4.js:2:16:2:24 | "tainted" | tst4.js:16:15:16:28 | p.also_tainted |
5050
| tst4.js:2:16:2:24 | "tainted" | tst4.js:17:15:17:28 | substr(source) |
51+
| tst6.mjs:12:14:12:21 | "source" | tst6.mjs:14:12:14:16 | a.m() |
5152
| tst.js:2:17:2:22 | "src1" | tst.js:3:15:3:29 | String(source1) |
5253
| tst.js:2:17:2:22 | "src1" | tst.js:4:15:4:29 | RegExp(source1) |
5354
| tst.js:2:17:2:22 | "src1" | tst.js:5:15:5:33 | new String(source1) |
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class A {
2+
constructor(f) {
3+
this._f = f;
4+
5+
}
6+
7+
m() {
8+
return this._f;
9+
}
10+
}
11+
12+
var source = "source";
13+
var a = new A(source);
14+
var sink = a.m();

0 commit comments

Comments
 (0)