Skip to content

Commit 55394df

Browse files
author
Max Schaefer
committed
JavaScript: Refactor HTTP libraries to use type tracking instead of tracked nodes.
1 parent 74db8b1 commit 55394df

File tree

9 files changed

+102
-87
lines changed

9 files changed

+102
-87
lines changed

‎javascript/ql/src/semmle/javascript/frameworks/Connect.qll‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,16 @@ module Connect {
118118
}
119119

120120
override DataFlow::SourceNode getARouteHandler() {
121-
result.(DataFlow::SourceNode).flowsTo(getARouteHandlerExpr().flow()) or
122-
result.(DataFlow::TrackedNode).flowsTo(getARouteHandlerExpr().flow())
121+
result = getARouteHandler(_)
122+
}
123+
124+
private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
125+
t.start() and
126+
result = getARouteHandlerExpr().flow().getALocalSource()
127+
or
128+
exists(DataFlow::TypeBackTracker next |
129+
result = getARouteHandler(next).backtrack(next, t)
130+
)
123131
}
124132

125133
override Expr getServer() { result = server }
@@ -169,24 +177,13 @@ module Connect {
169177
}
170178

171179
/**
172-
* Tracking for `RouteHandlerCandidate`.
173-
*/
174-
private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode {
175-
TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate }
176-
}
177-
178-
/**
179-
* A function that looks like a Connect route handler and flows to a route setup.
180+
* A function that flows to a route setup.
180181
*/
181182
private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler,
182-
HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
183-
override Function astNode;
183+
HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
184184

185185
TrackedRouteHandlerCandidateWithSetup() {
186-
exists(TrackedRouteHandlerCandidate tracked |
187-
tracked.flowsTo(any(RouteSetup s).getARouteHandlerExpr().flow()) and
188-
this = tracked
189-
)
186+
this = any(RouteSetup s).getARouteHandler()
190187
}
191188

192189
override SimpleParameter getRouteHandlerParameter(string kind) {

‎javascript/ql/src/semmle/javascript/frameworks/Express.qll‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,16 @@ module Express {
111111
Expr getLastRouteHandlerExpr() { result = max(int i | | getRouteHandlerExpr(i) order by i) }
112112

113113
override DataFlow::SourceNode getARouteHandler() {
114-
result.(DataFlow::SourceNode).flowsTo(getARouteHandlerExpr().flow()) or
115-
result.(DataFlow::TrackedNode).flowsTo(getARouteHandlerExpr().flow())
114+
result = getARouteHandler(_)
115+
}
116+
117+
private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
118+
t.start() and
119+
result = getARouteHandlerExpr().flow().getALocalSource()
120+
or
121+
exists(DataFlow::TypeBackTracker next |
122+
result = getARouteHandler(next).backtrack(next, t)
123+
)
116124
}
117125

118126
override Expr getServer() { result.(Application).getARouteHandler() = getARouteHandler() }
@@ -766,24 +774,13 @@ module Express {
766774
}
767775

768776
/**
769-
* Tracking for `RouteHandlerCandidate`.
770-
*/
771-
private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode {
772-
TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate }
773-
}
774-
775-
/**
776-
* A function that looks like an Express route handler and flows to a route setup.
777+
* A function that flows to a route setup.
777778
*/
778779
private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler,
779-
HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
780-
override Function astNode;
780+
HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
781781

782782
TrackedRouteHandlerCandidateWithSetup() {
783-
exists(TrackedRouteHandlerCandidate tracked |
784-
tracked.flowsTo(any(RouteSetup s).getARouteHandlerExpr().flow()) and
785-
this = tracked
786-
)
783+
this = any(RouteSetup s).getARouteHandler()
787784
}
788785

789786
override SimpleParameter getRouteHandlerParameter(string kind) {

‎javascript/ql/src/semmle/javascript/frameworks/HTTP.qll‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,23 +276,41 @@ module HTTP {
276276
* a request object enters the flow graph, such as the request
277277
* parameter of a route handler.
278278
*/
279-
abstract class RequestSource extends DataFlow::TrackedNode {
279+
abstract class RequestSource extends DataFlow::Node {
280280
/**
281281
* Gets the route handler that handles this request.
282282
*/
283283
abstract RouteHandler getRouteHandler();
284+
285+
predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) }
286+
287+
private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {
288+
t.start() and
289+
result = this
290+
or
291+
exists(DataFlow::TypeTracker prev | result = flowsToSourceNode(prev).track(prev, t))
292+
}
284293
}
285294

286295
/**
287296
* A response source, that is, a data flow node through which
288297
* a response object enters the flow graph, such as the response
289298
* parameter of a route handler.
290299
*/
291-
abstract class ResponseSource extends DataFlow::TrackedNode {
300+
abstract class ResponseSource extends DataFlow::Node {
292301
/**
293302
* Gets the route handler that provides this response.
294303
*/
295304
abstract RouteHandler getRouteHandler();
305+
306+
predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) }
307+
308+
private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {
309+
t.start() and
310+
result = this
311+
or
312+
exists(DataFlow::TypeTracker prev | result = flowsToSourceNode(prev).track(prev, t))
313+
}
296314
}
297315

298316
/**

‎javascript/ql/src/semmle/javascript/frameworks/Hapi.qll‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,16 @@ module Hapi {
193193
}
194194

195195
override DataFlow::SourceNode getARouteHandler() {
196-
result.(DataFlow::SourceNode).flowsTo(handler.flow()) or
197-
result.(DataFlow::TrackedNode).flowsTo(handler.flow())
196+
result = getARouteHandler(_)
197+
}
198+
199+
private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
200+
t.start() and
201+
result = handler.flow().getALocalSource()
202+
or
203+
exists(DataFlow::TypeBackTracker next |
204+
result = getARouteHandler(next).backtrack(next, t)
205+
)
198206
}
199207

200208
Expr getRouteHandlerExpr() { result = handler }
@@ -223,25 +231,13 @@ module Hapi {
223231
}
224232
}
225233

226-
/**
227-
* Tracking for `RouteHandlerCandidate`.
228-
*/
229-
private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode {
230-
TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate }
231-
}
232-
233234
/**
234235
* A function that looks like a Hapi route handler and flows to a route setup.
235236
*/
236237
private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler,
237-
HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
238-
override Function astNode;
239-
238+
HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
240239
TrackedRouteHandlerCandidateWithSetup() {
241-
exists(TrackedRouteHandlerCandidate tracked |
242-
tracked.flowsTo(any(RouteSetup s).getRouteHandlerExpr().flow()) and
243-
this = tracked
244-
)
240+
this = any(RouteSetup s).getARouteHandler()
245241
}
246242
}
247243
}

‎javascript/ql/src/semmle/javascript/frameworks/Koa.qll‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ module Koa {
6464
* A Koa context source, that is, the context parameter of a
6565
* route handler, or a `this` access in a route handler.
6666
*/
67-
private class ContextSource extends DataFlow::TrackedNode {
67+
private class ContextSource extends DataFlow::Node {
6868
RouteHandler rh;
6969

7070
ContextSource() {
@@ -77,6 +77,19 @@ module Koa {
7777
* Gets the route handler that handles this request.
7878
*/
7979
RouteHandler getRouteHandler() { result = rh }
80+
81+
predicate flowsTo(DataFlow::Node nd) {
82+
flowsToSourceNode(_).flowsTo(nd)
83+
}
84+
85+
private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {
86+
t.start() and
87+
result = this
88+
or
89+
exists(DataFlow::TypeTracker prev |
90+
result = flowsToSourceNode(prev).track(prev, t)
91+
)
92+
}
8093
}
8194

8295
/**

‎javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,16 @@ module NodeJSLib {
189189
}
190190

191191
override DataFlow::SourceNode getARouteHandler() {
192-
result.(DataFlow::SourceNode).flowsTo(handler.flow()) or
193-
result.(DataFlow::TrackedNode).flowsTo(handler.flow())
192+
result = getARouteHandler(_)
193+
}
194+
195+
private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
196+
t.start() and
197+
result = handler.flow().getALocalSource()
198+
or
199+
exists(DataFlow::TypeBackTracker next |
200+
result = getARouteHandler(next).backtrack(next, t)
201+
)
194202
}
195203

196204
override Expr getServer() { result = server }
@@ -613,24 +621,12 @@ module NodeJSLib {
613621
}
614622

615623
/**
616-
* Tracking for `RouteHandlerCandidate`.
617-
*/
618-
private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode {
619-
TrackedRouteHandlerCandidate() { this instanceof RouteHandlerCandidate }
620-
}
621-
622-
/**
623-
* A function that looks like a Node.js route handler and flows to a route setup.
624+
* A function that flows to a route setup.
624625
*/
625626
private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler,
626-
HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
627-
override Function astNode;
628-
627+
HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
629628
TrackedRouteHandlerCandidateWithSetup() {
630-
exists(TrackedRouteHandlerCandidate tracked |
631-
tracked.flowsTo(any(RouteSetup s).getRouteHandlerExpr().flow()) and
632-
this = tracked
633-
)
629+
this = any(RouteSetup s).getARouteHandler()
634630
}
635631
}
636632

‎javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandler.expected‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@
1111
| src/exported-handler.js:1:19:1:55 | functio ... res) {} |
1212
| src/exported-middleware-attacher-2.js:2:13:2:32 | function(req, res){} |
1313
| src/exported-middleware-attacher.js:2:13:2:32 | function(req, res){} |
14-
| src/handler-in-property.js:5:14:5:33 | function(req, res){} |
15-
| src/handler-in-property.js:12:18:12:37 | function(req, res){} |
1614
| src/middleware-attacher-getter.js:4:17:4:36 | function(req, res){} |
1715
| src/middleware-attacher-getter.js:19:19:19:38 | function(req, res){} |
1816
| src/middleware-attacher.js:3:13:3:32 | function(req, res){} |

‎javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,34 @@
11
| src/bound-handler.js:4:1:4:28 | functio ... res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
22
| src/bound-handler.js:9:12:9:31 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
3+
| src/handler-in-property.js:5:14:5:33 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
4+
| src/handler-in-property.js:12:18:12:37 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
35
| src/hapi.js:1:1:1:30 | functio ... t, h){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
46
| src/iterated-handlers.js:4:2:4:22 | functio ... res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
57
| src/middleware-attacher-getter.js:29:32:29:51 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
8+
| src/nodejs.js:5:22:5:41 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
9+
| src/nodejs.js:11:23:11:42 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
10+
| src/nodejs.js:12:25:12:44 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
611
| src/route-objects.js:7:19:7:38 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
712
| src/route-objects.js:8:12:10:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
813
| src/route-objects.js:20:16:22:9 | (req, r ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
914
| src/route-objects.js:27:16:29:9 | (req, r ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1015
| src/route-objects.js:40:12:42:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1116
| src/route-objects.js:50:12:52:5 | (req, res) {\\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1217
| src/route-objects.js:56:12:58:5 | functio ... ;\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
18+
| src/tst.js:6:32:6:52 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
19+
| src/tst.js:8:32:8:61 | functio ... nse) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
20+
| src/tst.js:30:36:30:56 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
21+
| src/tst.js:33:18:33:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
22+
| src/tst.js:34:18:34:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
23+
| src/tst.js:37:5:37:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
24+
| src/tst.js:38:5:38:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
25+
| src/tst.js:43:18:43:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1326
| src/tst.js:46:1:46:23 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1427
| src/tst.js:52:1:54:1 | functio ... req()\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1528
| src/tst.js:61:1:63:1 | functio ... turn;\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1629
| src/tst.js:70:5:72:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
30+
| src/tst.js:79:5:81:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
31+
| src/tst.js:84:5:86:5 | functio ... \\n\\n } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
1732
| src/tst.js:109:16:111:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
33+
| src/tst.js:124:16:126:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
34+
| src/tst.js:132:16:134:9 | functio ... } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |

‎javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteSetupCandidate.expected‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +0,0 @@
1-
| src/nodejs.js:5:1:5:42 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
2-
| src/nodejs.js:11:1:11:43 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
3-
| src/nodejs.js:12:1:12:45 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
4-
| src/tst.js:6:1:6:53 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
5-
| src/tst.js:8:1:8:62 | someOth ... se) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
6-
| src/tst.js:30:1:30:57 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
7-
| src/tst.js:32:1:34:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
8-
| src/tst.js:36:1:39:2 | someOth ... ) {}\\n]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
9-
| src/tst.js:41:1:43:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
10-
| src/tst.js:87:5:87:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
11-
| src/tst.js:96:5:96:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
12-
| src/tst.js:98:5:98:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
13-
| src/tst.js:104:5:104:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
14-
| src/tst.js:137:5:137:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
15-
| src/tst.js:149:5:149:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
16-
| src/tst.js:151:5:151:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
17-
| src/tst.js:157:5:157:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |

0 commit comments

Comments
 (0)