Skip to content

Commit 681b1a5

Browse files
yuskeyuske
committed
JS: Implement a call-sensitive data-flow analysis for .apply() call arguments
1 parent 970655d commit 681b1a5

File tree

6 files changed

+112
-35
lines changed

6 files changed

+112
-35
lines changed

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -404,20 +404,4 @@ private module ArrayLibraries {
404404
)
405405
}
406406
}
407-
408-
/**
409-
* A step modelling that a call of `.apply()` function with passing an array of arguments via 2nd parameter.
410-
*/
411-
private class ApplyCallStep extends DataFlow::SharedFlowStep {
412-
override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
413-
exists(DataFlow::MethodCallNode call, DataFlow::FunctionNode func, int i |
414-
call.getMethodName() = "apply" and
415-
call.getReceiver().getABoundFunctionValue(_) = func and
416-
prop = arrayElement(i) and
417-
not prop = arrayElement() and
418-
pred = call.getArgument(1) and
419-
succ = func.getParameter(i)
420-
)
421-
}
422-
}
423407
}

‎javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,14 +1161,46 @@ module DataFlow {
11611161
override NewExpr astNode;
11621162
}
11631163

1164+
/**
1165+
* A data flow node representing arguments of the `.apply` function call
1166+
* to emulate a separate argument for each parameter of a reflective function call.
1167+
*/
1168+
private class ApplyArgumentNode extends DataFlow::Node {
1169+
ExplicitMethodCallNode call;
1170+
Node arrayArgument;
1171+
int index;
1172+
1173+
ApplyArgumentNode() {
1174+
this = TApplyArgumentNode(
1175+
call.asExpr(),
1176+
call.getReceiver().getABoundFunctionValue(_).getFunction(),
1177+
index) and
1178+
arrayArgument = call.getArgument(1)
1179+
}
1180+
1181+
/**
1182+
* Gets an explicit call of the `.apply` function call
1183+
* that takes an argument represented by this data flow node.
1184+
* */
1185+
ExplicitMethodCallNode getCall() { result = call }
1186+
1187+
/** Gets an argument index represented by this data flow node. */
1188+
int getIndex() { result = index }
1189+
1190+
override string toString() { result = arrayArgument.toString() + "[" + index.toString() + "]" }
1191+
1192+
override predicate hasLocationInfo(
1193+
string filepath, int startline, int startcolumn, int endline, int endcolumn
1194+
) {
1195+
arrayArgument.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
1196+
}
1197+
}
1198+
11641199
/**
11651200
* A data flow node representing a reflective function call.
11661201
*/
1167-
private class ReflectiveCallNodeDef extends CallNodeDef {
1202+
private abstract class ReflectiveCallNodeDef extends CallNodeDef {
11681203
ExplicitMethodCallNode originalCall;
1169-
string kind;
1170-
1171-
ReflectiveCallNodeDef() { this = TReflectiveCallNode(originalCall.asExpr(), kind) }
11721204

11731205
override InvokeExpr getInvokeExpr() { result = originalCall.getInvokeExpr() }
11741206

@@ -1179,25 +1211,65 @@ module DataFlow {
11791211
override DataFlow::Node getCalleeNode() { result = originalCall.getReceiver() }
11801212

11811213
override DataFlow::Node getReceiver() { result = originalCall.getArgument(0) }
1214+
}
1215+
1216+
/**
1217+
* A data flow node representing a `.call` reflective function call.
1218+
*/
1219+
private class CallReflectiveCallNodeDef extends ReflectiveCallNodeDef {
1220+
CallReflectiveCallNodeDef() { this = TReflectiveCallNode(originalCall.asExpr(), "call") }
11821221

11831222
override DataFlow::Node getArgument(int i) {
1184-
i >= 0 and kind = "call" and result = originalCall.getArgument(i + 1)
1223+
i >= 0 and result = originalCall.getArgument(i + 1)
11851224
}
11861225

11871226
override DataFlow::Node getAnArgument() {
1188-
kind = "call" and result = originalCall.getAnArgument() and result != getReceiver()
1227+
result = originalCall.getAnArgument() and
1228+
result != getReceiver()
11891229
}
11901230

11911231
override DataFlow::Node getASpreadArgument() {
1192-
kind = "apply" and
1193-
result = originalCall.getArgument(1)
1194-
or
1195-
kind = "call" and
11961232
result = originalCall.getASpreadArgument()
11971233
}
11981234

11991235
override int getNumArgument() {
1200-
result >= 0 and kind = "call" and result = originalCall.getNumArgument() - 1
1236+
result >= 0 and result = originalCall.getNumArgument() - 1
1237+
}
1238+
}
1239+
1240+
/**
1241+
* A data flow node representing a `.apply` reflective function call.
1242+
*/
1243+
class ApplyReflectiveCallNodeDef extends ReflectiveCallNodeDef {
1244+
ApplyReflectiveCallNodeDef() { this = TReflectiveCallNode(originalCall.asExpr(), "apply") }
1245+
1246+
ApplyArgumentNode getApplyArgument(int i) {
1247+
result.getCall() = originalCall and
1248+
result.getIndex() = i
1249+
}
1250+
1251+
override DataFlow::Node getArgument(int i) { none() }
1252+
1253+
override DataFlow::Node getAnArgument() { none() }
1254+
1255+
override DataFlow::Node getASpreadArgument() {
1256+
result = originalCall.getArgument(1)
1257+
}
1258+
1259+
override int getNumArgument() { none() }
1260+
}
1261+
1262+
/**
1263+
* A step modelling that a call of `.apply()` function with passing an array of arguments via 2nd parameter.
1264+
*/
1265+
private class ApplyCallStep extends PreCallGraphStep {
1266+
override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
1267+
exists(ApplyReflectiveCallNodeDef call, int i |
1268+
prop = DataFlow::PseudoProperties::arrayElement(i) and
1269+
not prop = DataFlow::PseudoProperties::arrayElement() and
1270+
pred = call.getASpreadArgument().getALocalSource() and
1271+
succ = call.getApplyArgument(i)
1272+
)
12011273
}
12021274
}
12031275
}

‎javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowNode.qll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,7 @@ newtype TNode =
3131
TExceptionalFunctionReturnNode(Function f) or
3232
TExceptionalInvocationReturnNode(InvokeExpr e) or
3333
TGlobalAccessPathRoot() or
34-
TTemplatePlaceholderTag(Templating::TemplatePlaceholderTag tag)
34+
TTemplatePlaceholderTag(Templating::TemplatePlaceholderTag tag) or
35+
TApplyArgumentNode(MethodCallExpr ce, Function func, int i) {
36+
exists(func.getParameter(i))
37+
}

‎javascript/ql/lib/semmle/javascript/dataflow/internal/FlowSteps.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ private module CachedSteps {
204204
) {
205205
calls(invk, f) and
206206
(
207-
exists(int i | arg = invk.(DataFlow::InvokeNode).getArgument(i) |
207+
exists(int i | arg = invk.(DataFlow::InvokeNode).getArgument(i) or arg = invk.(DataFlow::Impl::ApplyReflectiveCallNodeDef).getApplyArgument(i) |
208208
exists(Parameter p |
209209
f.getParameter(i) = p and
210210
not p.isRestParameter() and

‎javascript/ql/test/library-tests/InterProceduralFlow/call-apply.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ function foo2(arg1, arg2) {
66
return arg2;
77
}
88

9-
function foo11(arr) {
9+
function foo1_apply(arr) {
1010
return foo1.apply(this, arr);
1111
}
1212

13+
function foo1_call(arr) {
14+
return foo1.call(this, arr[0], arr[1]);
15+
}
16+
17+
1318
var source = "source";
1419

1520
var sink0 = foo1.call(null, source, ""); // OK
@@ -18,5 +23,8 @@ var sink1 = foo2.call(null, source, ""); // NOT OK
1823
var sink2 = foo1.apply(null, [source, ""]); // OK
1924
var sink3 = foo2.apply(null, [source, ""]); // NOT OK
2025

21-
var sink4 = foo11([source, ""]); // OK
22-
var sink5 = foo11(["", source]); // NOT OK
26+
var sink4 = foo1_apply([source, ""]); // OK
27+
var sink5 = foo1_apply(["", source]); // NOT OK
28+
29+
var sink6 = foo1_call([source, ""]); // OK
30+
var sink7 = foo1_call(["", source]); // NOT OK

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ dataFlow
1111
| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await ... ce))).p |
1212
| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
1313
| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
14-
| call-apply.js:13:14:13:21 | "source" | call-apply.js:15:13:15:39 | foo1.ca ... ce, "") |
14+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:20:13:20:39 | foo1.ca ... ce, "") |
15+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:23:13:23:42 | foo1.ap ... e, ""]) |
16+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:26:13:26:36 | foo1_ap ... e, ""]) |
17+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:29:13:29:35 | foo1_ca ... e, ""]) |
1518
| callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
1619
| callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
1720
| callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
@@ -94,7 +97,11 @@ taintTracking
9497
| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await ... ce))).p |
9598
| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
9699
| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
97-
| call-apply.js:13:14:13:21 | "source" | call-apply.js:15:13:15:39 | foo1.ca ... ce, "") |
100+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:20:13:20:39 | foo1.ca ... ce, "") |
101+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:23:13:23:42 | foo1.ap ... e, ""]) |
102+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:26:13:26:36 | foo1_ap ... e, ""]) |
103+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:29:13:29:35 | foo1_ca ... e, ""]) |
104+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:30:13:30:35 | foo1_ca ... ource]) |
98105
| callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
99106
| callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
100107
| callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |
@@ -200,7 +207,10 @@ germanFlow
200207
| async.js:79:16:79:23 | "source" | async.js:80:14:80:36 | (await ... ce))).p |
201208
| async.js:79:16:79:23 | "source" | async.js:92:15:92:30 | await (getP(o3)) |
202209
| async.js:96:18:96:25 | "source" | async.js:101:15:101:27 | await readP() |
203-
| call-apply.js:13:14:13:21 | "source" | call-apply.js:15:13:15:39 | foo1.ca ... ce, "") |
210+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:20:13:20:39 | foo1.ca ... ce, "") |
211+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:23:13:23:42 | foo1.ap ... e, ""]) |
212+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:26:13:26:36 | foo1_ap ... e, ""]) |
213+
| call-apply.js:18:14:18:21 | "source" | call-apply.js:29:13:29:35 | foo1_ca ... e, ""]) |
204214
| callback.js:16:14:16:21 | "source" | callback.js:13:14:13:14 | x |
205215
| callback.js:17:15:17:23 | "source2" | callback.js:13:14:13:14 | x |
206216
| callback.js:27:15:27:23 | "source3" | callback.js:13:14:13:14 | x |

0 commit comments

Comments
 (0)