Skip to content

Commit 39c37f5

Browse files
emartecaMax Schaefer
authored andcommitted
JavaScript: Use type tracking to identify more portal entry/exit nodes.
1 parent 9bf0a3f commit 39c37f5

File tree

6 files changed

+98
-10
lines changed

6 files changed

+98
-10
lines changed

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

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,37 @@ class Portal extends TPortal {
115115
abstract int depth();
116116
}
117117

118+
/**
119+
* Gets an exit node for the specified portal, using TypeTracking.
120+
*
121+
* Called in instances where an exit node is computed from another exit node
122+
* (for example, in MemberPortal's portalBaseRef predicate).
123+
*/
124+
private DataFlow::SourceNode trackExitNode(
125+
Portal base, boolean isRemote, DataFlow::TypeTracker t
126+
) {
127+
t.start() and
128+
result = base.getAnExitNode(isRemote)
129+
or
130+
exists(DataFlow::TypeTracker t2 | result = trackExitNode(base, isRemote, t2).track(t2, t))
131+
}
132+
133+
/**
134+
* Gets an entry node for the specified portal, using TypeBackTracking.
135+
*
136+
* Parallel to trackExitNode above.
137+
*/
138+
private DataFlow::SourceNode trackEntryNode(
139+
Portal base, boolean escapes, DataFlow::TypeBackTracker t
140+
) {
141+
t.start() and
142+
result = base.getAnEntryNode(escapes).getALocalSource()
143+
or
144+
exists(DataFlow::TypeBackTracker t2 |
145+
result = trackEntryNode(base, escapes, t2).backtrack(t2, t)
146+
)
147+
}
148+
118149
/**
119150
* A portal representing the exports value of the main module of an npm
120151
* package (that is, a value of `module.exports` for CommonJS modules, or
@@ -260,9 +291,9 @@ private class MemberPortal extends CompoundPortal, MkMemberPortal {
260291
private module MemberPortal {
261292
/** Gets a node representing a value flowing through `base`, that is, either an entry node or an exit node. */
262293
private DataFlow::SourceNode portalBaseRef(Portal base, boolean escapes) {
263-
result = base.getAnExitNode(escapes)
294+
result = trackExitNode(base, escapes, DataFlow::TypeTracker::end())
264295
or
265-
result = base.getAnEntryNode(escapes).getALocalSource()
296+
result = trackEntryNode(base, escapes, DataFlow::TypeBackTracker::end())
266297
}
267298

268299
/** Holds if `read` is a read of property `prop` of a value flowing through `base`. */
@@ -342,13 +373,13 @@ private module InstancePortal {
342373
Portal base, DataFlow::SourceNode ctor, AbstractInstance i, boolean escapes
343374
) {
344375
ctor = DataFlow::valueNode(i.getConstructor().getDefinition()) and
345-
ctor.flowsTo(base.getAnEntryNode(escapes)) and
376+
ctor = trackEntryNode(base, escapes, DataFlow::TypeBackTracker::end()) and
346377
instantiable(ctor)
347378
}
348379

349380
/** Holds if `nd` is an expression evaluating to an instance of `base`. */
350381
predicate instanceUse(Portal base, DataFlow::SourceNode nd, boolean isRemote) {
351-
nd = base.getAnExitNode(isRemote).getAnInstantiation()
382+
nd = trackExitNode(base, isRemote, DataFlow::TypeTracker::end()).getAnInstantiation()
352383
or
353384
isInstance(base, _, nd.analyze().getAValue(), isRemote)
354385
}
@@ -415,13 +446,14 @@ class ParameterPortal extends CompoundPortal, MkParameterPortal {
415446
private module ParameterPortal {
416447
/** Holds if `param` is the `i`th parameter of a function flowing through `base`. */
417448
predicate parameter(Portal base, int i, DataFlow::SourceNode param, boolean isRemote) {
418-
param = base.getAnEntryNode(isRemote).getALocalSource().(DataFlow::FunctionNode).getParameter(i)
449+
param = trackEntryNode(base, isRemote, DataFlow::TypeBackTracker::end()).(DataFlow::FunctionNode)
450+
.getParameter(i)
419451
}
420452

421453
/** Holds if `arg` is the `i`th argument passed to an invocation of a function flowing through `base`. */
422454
predicate argument(Portal base, int i, DataFlow::Node arg, boolean escapes) {
423455
exists(DataFlow::InvokeNode invk |
424-
invk = base.getAnExitNode(escapes).getAnInvocation() and
456+
invk = trackExitNode(base, escapes, DataFlow::TypeTracker::end()).getAnInvocation() and
425457
arg = invk.getArgument(i)
426458
)
427459
}
@@ -449,11 +481,14 @@ class ReturnPortal extends CompoundPortal, MkReturnPortal {
449481
private module ReturnPortal {
450482
/** Holds if `invk` is a call to a function flowing through `callee`. */
451483
predicate calls(DataFlow::InvokeNode invk, Portal callee, boolean isRemote) {
452-
invk = callee.getAnExitNode(isRemote).getAnInvocation()
484+
invk = trackExitNode(callee, isRemote, DataFlow::TypeTracker::end())
485+
.getAnInvocation()
453486
}
454487

455488
/** Holds if `ret` is a return node of a function flowing through `callee`. */
456489
predicate returns(Portal base, DataFlow::Node ret, boolean escapes) {
457-
ret = base.getAnEntryNode(escapes).getALocalSource().(DataFlow::FunctionNode).getAReturn()
490+
ret = trackEntryNode(base, escapes, DataFlow::TypeBackTracker::end())
491+
.(DataFlow::FunctionNode)
492+
.getAReturn()
458493
}
459494
}

javascript/ql/test/library-tests/Portals/PortalEntry.expected

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
| (member NP (root https://www.npmjs.com/package/typeTracking)) | src/typeTracking/index.js:20:14:20:15 | NP | true |
12
| (member Promise (root https://www.npmjs.com/package/bluebird)) | src/bluebird/index.js:9:19:9:25 | Promise | true |
3+
| (member argFunc (instance (member NP (root https://www.npmjs.com/package/typeTracking)))) | src/typeTracking/index.js:16:24:18:1 | functio ... ck();\\n} | true |
4+
| (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking)))) | src/typeTracking/index.js:12:25:14:1 | functio ... bj();\\n} | true |
25
| (member default (root https://www.npmjs.com/package/m2)) | src/m2/main.js:6:16:16:1 | class { ... y; }\\n} | true |
6+
| (member exec (instance (member NP (root https://www.npmjs.com/package/typeTracking)))) | src/typeTracking/index.js:2:14:2:17 | exec | true |
37
| (member exec (instance (member Promise (root https://www.npmjs.com/package/bluebird)))) | src/bluebird/index.js:2:15:2:18 | exec | true |
48
| (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:5:11:5:13 | foo | true |
59
| (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:5:11:5:13 | foo | true |
@@ -1014,6 +1018,7 @@
10141018
| (parameter 0 (root https://www.npmjs.com/package/m1)) | src/m3/index.js:3:46:3:60 | "Hello, world!" | false |
10151019
| (parameter 0 (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:7:4:10 | "me" | false |
10161020
| (parameter 0 (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:7:5:10 | "me" | false |
1021+
| (return (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking))))) | src/typeTracking/index.js:13:9:13:21 | new tempObj() | true |
10171022
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
10181023
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
10191024
| (return (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
@@ -1712,4 +1717,4 @@
17121717
| (return (return (return (return (return (return (return (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
17131718
| (return (return (return (return (return (return (return (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:3:10:3:12 | foo | true |
17141719
| (return (root https://www.npmjs.com/package/m1)) | src/m1/index.js:1:25:1:25 | x | true |
1715-
| (root https://www.npmjs.com/package/m1) | src/m1/index.js:1:18:1:25 | (x) => x | true |
1720+
| (root https://www.npmjs.com/package/m1) | src/m1/index.js:1:18:1:25 | (x) => x | true |

javascript/ql/test/library-tests/Portals/PortalExit.expected

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:1:1:1:0 | this | true |
2+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:12:1:12:12 | NP.prototype | true |
3+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:12:25:12:24 | this | true |
4+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:16:1:16:12 | NP.prototype | true |
5+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:16:24:16:23 | this | true |
6+
| (instance (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/nonlocalDataflowRead.js:4:10:4:16 | new n() | true |
17
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:1:1:1:0 | this | true |
28
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:5:1:5:17 | Promise.prototype | true |
39
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:5:26:5:25 | this | true |
@@ -9,6 +15,8 @@
915
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
1016
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
1117
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
18+
| (member argFunc (return (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking)))))) | src/typeTracking/nonlocalDataflowRead.js:11:3:11:27 | getPort ... argFunc | true |
19+
| (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking)))) | src/typeTracking/nonlocalDataflowRead.js:7:9:7:19 | np.createNP | true |
1220
| (member default (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:1:8:1:8 | A | false |
1321
| (member foo (root https://www.npmjs.com/package/m2)) | src/m3/tst2.js:1:10:1:12 | foo | false |
1422
| (member m (instance (member default (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:4:1:4:13 | new A("me").m | false |
@@ -26,7 +34,9 @@
2634
| (member s (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:3:1:3:3 | A.s | false |
2735
| (member x (parameter 0 (member foo (root https://www.npmjs.com/package/m2)))) | src/m2/main.js:2:15:2:17 | p.x | true |
2836
| (member y (member x (parameter 0 (member foo (root https://www.npmjs.com/package/m2))))) | src/m2/main.js:2:15:2:19 | p.x.y | true |
37+
| (parameter 0 (member NP (root https://www.npmjs.com/package/typeTracking))) | src/typeTracking/index.js:1:13:1:16 | exec | true |
2938
| (parameter 0 (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:1:18:1:21 | exec | true |
39+
| (parameter 0 (member argFunc (instance (member NP (root https://www.npmjs.com/package/typeTracking))))) | src/typeTracking/index.js:16:33:16:44 | someCallback | true |
3040
| (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:1:14:1:15 | cb | true |
3141
| (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:1:14:1:15 | cb | true |
3242
| (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:1:14:1:15 | cb | true |
@@ -730,6 +740,8 @@
730740
| (parameter 0 (return (return (return (return (return (return (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:1:14:1:15 | cb | true |
731741
| (parameter 0 (root https://www.npmjs.com/package/m1)) | src/m1/index.js:1:19:1:19 | x | true |
732742
| (parameter 1 (member then (instance (member Promise (root https://www.npmjs.com/package/bluebird))))) | src/bluebird/index.js:5:46:5:53 | rejected | true |
743+
| (return (member argFunc (return (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking))))))) | src/typeTracking/nonlocalDataflowRead.js:11:3:11:29 | getPort ... gFunc() | true |
744+
| (return (member createNP (instance (member NP (root https://www.npmjs.com/package/typeTracking))))) | src/typeTracking/nonlocalDataflowRead.js:7:9:7:21 | np.createNP() | true |
733745
| (return (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
734746
| (return (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
735747
| (return (member foo (root https://www.npmjs.com/package/m2))) | src/m3/tst2.js:5:1:5:13 | foo({ x: o }) | false |
@@ -745,6 +757,7 @@
745757
| (return (member s (return (member default (root https://www.npmjs.com/package/m2))))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
746758
| (return (member s (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
747759
| (return (member s (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:3:1:3:12 | A.s("there") | false |
760+
| (return (parameter 0 (member argFunc (instance (member NP (root https://www.npmjs.com/package/typeTracking)))))) | src/typeTracking/index.js:17:2:17:15 | someCallback() | true |
748761
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
749762
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (member foo (root https://www.npmjs.com/package/cyclic))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
750763
| (return (parameter 0 (member f00 (member f00 (member f00 (member f00 (member f00 (return (member foo (root https://www.npmjs.com/package/cyclic)))))))))) | src/cyclic/index.js:2:3:2:9 | cb(foo) | true |
@@ -1040,4 +1053,4 @@
10401053
| (root https://www.npmjs.com/package/m1) | src/m3/index.js:1:10:1:22 | require("m1") | false |
10411054
| (root https://www.npmjs.com/package/m2) | src/m3/tst2.js:1:1:1:25 | import ... m "m2"; | false |
10421055
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:1:1:19 | import A from "m2"; | false |
1043-
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |
1056+
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
function NP(exec) {
2+
this.exec = exec
3+
}
4+
5+
function tempObj() {
6+
}
7+
8+
tempObj.prototype.f = function(someCallback) {
9+
someCallback();
10+
};
11+
12+
NP.prototype.createNP = function() {
13+
return new tempObj();
14+
};
15+
16+
NP.prototype.argFunc = function(someCallback) {
17+
someCallback();
18+
}
19+
20+
exports.NP = NP;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
2+
3+
var n = require('./index').NP;
4+
var np = new n();
5+
6+
function getPortalReturn() {
7+
return np.createNP();
8+
}
9+
10+
function main() {
11+
getPortalReturn().argFunc(); // nonlocal property access
12+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "typeTracking"
3+
}

0 commit comments

Comments
 (0)