Skip to content

Commit 5944ec6

Browse files
authored
Merge pull request #1592 from Semmle/revert-1538-TypeTrackingInPortals
Approved by asger-semmle
2 parents 87a4371 + ca36c7a commit 5944ec6

File tree

6 files changed

+10
-98
lines changed

6 files changed

+10
-98
lines changed

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

Lines changed: 8 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -115,37 +115,6 @@ 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-
149118
/**
150119
* A portal representing the exports value of the main module of an npm
151120
* package (that is, a value of `module.exports` for CommonJS modules, or
@@ -291,9 +260,9 @@ private class MemberPortal extends CompoundPortal, MkMemberPortal {
291260
private module MemberPortal {
292261
/** Gets a node representing a value flowing through `base`, that is, either an entry node or an exit node. */
293262
private DataFlow::SourceNode portalBaseRef(Portal base, boolean escapes) {
294-
result = trackExitNode(base, escapes, DataFlow::TypeTracker::end())
263+
result = base.getAnExitNode(escapes)
295264
or
296-
result = trackEntryNode(base, escapes, DataFlow::TypeBackTracker::end())
265+
result = base.getAnEntryNode(escapes).getALocalSource()
297266
}
298267

299268
/** Holds if `read` is a read of property `prop` of a value flowing through `base`. */
@@ -373,13 +342,13 @@ private module InstancePortal {
373342
Portal base, DataFlow::SourceNode ctor, AbstractInstance i, boolean escapes
374343
) {
375344
ctor = DataFlow::valueNode(i.getConstructor().getDefinition()) and
376-
ctor = trackEntryNode(base, escapes, DataFlow::TypeBackTracker::end()) and
345+
ctor.flowsTo(base.getAnEntryNode(escapes)) and
377346
instantiable(ctor)
378347
}
379348

380349
/** Holds if `nd` is an expression evaluating to an instance of `base`. */
381350
predicate instanceUse(Portal base, DataFlow::SourceNode nd, boolean isRemote) {
382-
nd = trackExitNode(base, isRemote, DataFlow::TypeTracker::end()).getAnInstantiation()
351+
nd = base.getAnExitNode(isRemote).getAnInstantiation()
383352
or
384353
isInstance(base, _, nd.analyze().getAValue(), isRemote)
385354
}
@@ -446,14 +415,13 @@ class ParameterPortal extends CompoundPortal, MkParameterPortal {
446415
private module ParameterPortal {
447416
/** Holds if `param` is the `i`th parameter of a function flowing through `base`. */
448417
predicate parameter(Portal base, int i, DataFlow::SourceNode param, boolean isRemote) {
449-
param = trackEntryNode(base, isRemote, DataFlow::TypeBackTracker::end()).(DataFlow::FunctionNode)
450-
.getParameter(i)
418+
param = base.getAnEntryNode(isRemote).getALocalSource().(DataFlow::FunctionNode).getParameter(i)
451419
}
452420

453421
/** Holds if `arg` is the `i`th argument passed to an invocation of a function flowing through `base`. */
454422
predicate argument(Portal base, int i, DataFlow::Node arg, boolean escapes) {
455423
exists(DataFlow::InvokeNode invk |
456-
invk = trackExitNode(base, escapes, DataFlow::TypeTracker::end()).getAnInvocation() and
424+
invk = base.getAnExitNode(escapes).getAnInvocation() and
457425
arg = invk.getArgument(i)
458426
)
459427
}
@@ -481,14 +449,11 @@ class ReturnPortal extends CompoundPortal, MkReturnPortal {
481449
private module ReturnPortal {
482450
/** Holds if `invk` is a call to a function flowing through `callee`. */
483451
predicate calls(DataFlow::InvokeNode invk, Portal callee, boolean isRemote) {
484-
invk = trackExitNode(callee, isRemote, DataFlow::TypeTracker::end())
485-
.getAnInvocation()
452+
invk = callee.getAnExitNode(isRemote).getAnInvocation()
486453
}
487454

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

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
| (member NP (root https://www.npmjs.com/package/typeTracking)) | src/typeTracking/index.js:20:14:20:15 | NP | true |
21
| (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 |
52
| (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 |
73
| (member exec (instance (member Promise (root https://www.npmjs.com/package/bluebird)))) | src/bluebird/index.js:2:15:2:18 | exec | true |
84
| (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 |
95
| (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 |
@@ -1018,7 +1014,6 @@
10181014
| (parameter 0 (root https://www.npmjs.com/package/m1)) | src/m3/index.js:3:46:3:60 | "Hello, world!" | false |
10191015
| (parameter 0 (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:7:4:10 | "me" | false |
10201016
| (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 |
10221017
| (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 |
10231018
| (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 |
10241019
| (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 |
@@ -1717,4 +1712,4 @@
17171712
| (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 |
17181713
| (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 |
17191714
| (return (root https://www.npmjs.com/package/m1)) | src/m1/index.js:1:25:1:25 | x | true |
1720-
| (root https://www.npmjs.com/package/m1) | src/m1/index.js:1:18:1:25 | (x) => x | true |
1715+
| (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: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
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 |
71
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:1:1:1:0 | this | true |
82
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:5:1:5:17 | Promise.prototype | true |
93
| (instance (member Promise (root https://www.npmjs.com/package/bluebird))) | src/bluebird/index.js:5:26:5:25 | this | true |
@@ -15,8 +9,6 @@
159
| (instance (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
1610
| (instance (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
1711
| (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 |
2012
| (member default (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:1:8:1:8 | A | false |
2113
| (member foo (root https://www.npmjs.com/package/m2)) | src/m3/tst2.js:1:10:1:12 | foo | false |
2214
| (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 |
@@ -34,9 +26,7 @@
3426
| (member s (root https://www.npmjs.com/package/m2)) | src/m3/tst3.js:3:1:3:3 | A.s | false |
3527
| (member x (parameter 0 (member foo (root https://www.npmjs.com/package/m2)))) | src/m2/main.js:2:15:2:17 | p.x | true |
3628
| (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 |
3829
| (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 |
4030
| (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 |
4131
| (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 |
4232
| (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 |
@@ -740,8 +730,6 @@
740730
| (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 |
741731
| (parameter 0 (root https://www.npmjs.com/package/m1)) | src/m1/index.js:1:19:1:19 | x | true |
742732
| (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 |
745733
| (return (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:4:1:4:11 | new A("me") | false |
746734
| (return (member default (root https://www.npmjs.com/package/m2))) | src/m3/tst3.js:5:1:5:11 | new A("me") | false |
747735
| (return (member foo (root https://www.npmjs.com/package/m2))) | src/m3/tst2.js:5:1:5:13 | foo({ x: o }) | false |
@@ -757,7 +745,6 @@
757745
| (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 |
758746
| (return (member s (return (root https://www.npmjs.com/package/m2)))) | src/m3/tst3.js:5:1:5:22 | new A(" ... there") | false |
759747
| (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 |
761748
| (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 |
762749
| (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 |
763750
| (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 |
@@ -1053,4 +1040,4 @@
10531040
| (root https://www.npmjs.com/package/m1) | src/m3/index.js:1:10:1:22 | require("m1") | false |
10541041
| (root https://www.npmjs.com/package/m2) | src/m3/tst2.js:1:1:1:25 | import ... m "m2"; | false |
10551042
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:1:1:19 | import A from "m2"; | false |
1056-
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |
1043+
| (root https://www.npmjs.com/package/m2) | src/m3/tst3.js:1:8:1:8 | A | false |

javascript/ql/test/library-tests/Portals/src/typeTracking/index.js

Lines changed: 0 additions & 20 deletions
This file was deleted.

javascript/ql/test/library-tests/Portals/src/typeTracking/nonlocalDataflowRead.js

Lines changed: 0 additions & 12 deletions
This file was deleted.

javascript/ql/test/library-tests/Portals/src/typeTracking/package.json

Lines changed: 0 additions & 3 deletions
This file was deleted.

0 commit comments

Comments
 (0)