Skip to content

Commit 136124f

Browse files
erik-krogherik-krogh
committed
convert the remaining Koa models to DataFlow nodes
1 parent fc54ba8 commit 136124f

File tree

9 files changed

+97
-55
lines changed

9 files changed

+97
-55
lines changed

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

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ module Koa {
2424

2525
HeaderDefinition() {
2626
// ctx.set('Cache-Control', 'no-cache');
27-
this.calls(rh.getAResponseOrContextExpr().flow(), "set")
27+
this.calls(rh.getAResponseOrContextNode(), "set")
2828
or
2929
// ctx.response.header('Cache-Control', 'no-cache')
3030
this.calls(rh.getAResponseNode(), "header")
@@ -40,10 +40,17 @@ module Koa {
4040
/**
4141
* Gets the parameter of the route handler that contains the context object.
4242
*/
43-
Parameter getContextParameter() {
44-
result = this.getAFunctionValue().getFunction().getParameter(0)
43+
DataFlow::ParameterNode getContextParameter() {
44+
result = this.getAFunctionValue().getParameter(0)
4545
}
4646

47+
/**
48+
* DEPRECATED: Use `getAContextNode` instead.
49+
* Gets an expression that contains the "context" object of
50+
* a route handler invocation.
51+
*/
52+
deprecated Expr getAContextExpr() { result.(ContextExpr).getRouteHandler() = this }
53+
4754
/**
4855
* Gets an expression that contains the "context" object of
4956
* a route handler invocation.
@@ -52,26 +59,42 @@ module Koa {
5259
* `this` or `ctx`, given as the first and only argument to the
5360
* route handler.
5461
*/
55-
Expr getAContextExpr() { result.(ContextExpr).getRouteHandler() = this }
62+
DataFlow::Node getAContextNode() { result.(ContextNode).getRouteHandler() = this }
5663

5764
/**
65+
* DEPRECATED: Use `getAResponseOrContextNode` instead.
5866
* Gets an expression that contains the context or response
5967
* object of a route handler invocation.
6068
*/
61-
Expr getAResponseOrContextExpr() {
62-
// TODO: DataFlow::Node
69+
deprecated Expr getAResponseOrContextExpr() {
6370
result = this.getAResponseNode().asExpr() or result = this.getAContextExpr()
6471
}
6572

6673
/**
74+
* Gets an expression that contains the context or response
75+
* object of a route handler invocation.
76+
*/
77+
DataFlow::Node getAResponseOrContextNode() {
78+
result = this.getAResponseNode() or result = this.getAContextNode()
79+
}
80+
81+
/**
82+
* DEPRECATED: Use `getARequestOrContextNode` instead.
6783
* Gets an expression that contains the context or request
6884
* object of a route handler invocation.
6985
*/
70-
Expr getARequestOrContextExpr() {
71-
// TODO: DataFlow::Node
86+
deprecated Expr getARequestOrContextExpr() {
7287
result = this.getARequestNode().asExpr() or result = this.getAContextExpr()
7388
}
7489

90+
/**
91+
* Gets an expression that contains the context or request
92+
* object of a route handler invocation.
93+
*/
94+
DataFlow::Node getARequestOrContextNode() {
95+
result = this.getARequestNode() or result = this.getAContextNode()
96+
}
97+
7598
/**
7699
* Gets a reference to a request parameter defined by this route handler.
77100
*/
@@ -110,7 +133,7 @@ module Koa {
110133
RouteHandler rh;
111134

112135
ContextSource() {
113-
this = DataFlow::parameterNode(rh.getContextParameter())
136+
this = rh.getContextParameter()
114137
or
115138
this.(DataFlow::ThisNode).getBinder() = rh
116139
}
@@ -206,10 +229,10 @@ module Koa {
206229
* A Koa request source, that is, an access to the `request` property
207230
* of a context object.
208231
*/
209-
private class RequestSource extends HTTP::Servers::RequestSource {
210-
ContextExpr ctx;
232+
private class RequestSource extends HTTP::Servers::RequestSource instanceof DataFlow::PropRead {
233+
ContextNode ctx;
211234

212-
RequestSource() { this.asExpr().(PropAccess).accesses(ctx, "request") }
235+
RequestSource() { super.accesses(ctx, "request") }
213236

214237
/**
215238
* Gets the route handler that provides this response.
@@ -241,24 +264,37 @@ module Koa {
241264
* A Koa response source, that is, an access to the `response` property
242265
* of a context object.
243266
*/
244-
private class ResponseSource extends HTTP::Servers::ResponseSource {
245-
ContextExpr ctx;
267+
private class ResponseSource extends HTTP::Servers::ResponseSource instanceof DataFlow::PropRead {
268+
ContextNode ctx;
246269

247-
ResponseSource() { this.asExpr().(PropAccess).accesses(ctx, "response") }
270+
ResponseSource() { super.accesses(ctx, "response") }
248271

249272
/**
250273
* Gets the route handler that provides this response.
251274
*/
252275
override RouteHandler getRouteHandler() { result = ctx.getRouteHandler() }
253276
}
254277

278+
/**
279+
* DEPRECATED: Use `ContextNode` instead.
280+
* An expression that may hold a Koa context object.
281+
*/
282+
deprecated class ContextExpr extends Expr {
283+
ContextNode node;
284+
285+
ContextExpr() { node.asExpr() = this }
286+
287+
/** Gets the route handler that provides this response. */
288+
deprecated RouteHandler getRouteHandler() { result = node.getRouteHandler() }
289+
}
290+
255291
/**
256292
* An expression that may hold a Koa context object.
257293
*/
258-
class ContextExpr extends Expr {
294+
class ContextNode extends DataFlow::Node {
259295
ContextSource src;
260296

261-
ContextExpr() { src.ref().flowsTo(DataFlow::valueNode(this)) }
297+
ContextNode() { src.ref().flowsTo(this) }
262298

263299
/**
264300
* Gets the route handler that provides this response.
@@ -310,11 +346,11 @@ module Koa {
310346
kind = "parameter" and
311347
this = rh.getARequestParameterAccess()
312348
or
313-
exists(Expr e | rh.getARequestOrContextExpr() = e |
349+
exists(DataFlow::Node e | rh.getARequestOrContextNode() = e |
314350
// `ctx.request.url`, `ctx.request.originalUrl`, or `ctx.request.href`
315351
exists(string propName |
316352
kind = "url" and
317-
this.asExpr().(PropAccess).accesses(e, propName)
353+
this.(DataFlow::PropRead).accesses(e, propName)
318354
|
319355
propName = "url"
320356
or
@@ -325,19 +361,19 @@ module Koa {
325361
or
326362
// params, when handler is registered by `koa-router` or similar.
327363
kind = "parameter" and
328-
this.asExpr().(PropAccess).accesses(e, "params")
364+
this.(DataFlow::PropRead).accesses(e, "params")
329365
or
330366
// `ctx.request.body`
331-
e.flow() instanceof RequestNode and
367+
e instanceof RequestNode and
332368
kind = "body" and
333-
this.asExpr().(PropAccess).accesses(e, "body")
369+
this.(DataFlow::PropRead).accesses(e, "body")
334370
or
335371
// `ctx.cookies.get(<name>)`
336-
exists(PropAccess cookies |
337-
e instanceof ContextExpr and
372+
exists(DataFlow::PropRead cookies |
373+
e instanceof ContextNode and
338374
kind = "cookie" and
339375
cookies.accesses(e, "cookies") and
340-
this = cookies.flow().(DataFlow::SourceNode).getAMethodCall("get")
376+
this = cookies.getAMethodCall("get")
341377
)
342378
or
343379
exists(RequestHeaderAccess access | access = this |
@@ -356,9 +392,9 @@ module Koa {
356392

357393
private DataFlow::Node getAQueryParameterAccess(RouteHandler rh) {
358394
// `ctx.query.name` or `ctx.request.query.name`
359-
exists(PropAccess q |
360-
q.accesses(rh.getARequestOrContextExpr(), "query") and
361-
result = q.flow().(DataFlow::SourceNode).getAPropertyRead()
395+
exists(DataFlow::PropRead q |
396+
q.accesses(rh.getARequestOrContextNode(), "query") and
397+
result = q.getAPropertyRead()
362398
)
363399
}
364400

@@ -369,18 +405,18 @@ module Koa {
369405
RouteHandler rh;
370406

371407
RequestHeaderAccess() {
372-
exists(Expr e | e = rh.getARequestOrContextExpr() |
373-
exists(string propName, PropAccess headers |
408+
exists(DataFlow::Node e | e = rh.getARequestOrContextNode() |
409+
exists(string propName, DataFlow::PropRead headers |
374410
// `ctx.request.header.<name>`, `ctx.request.headers.<name>`
375411
headers.accesses(e, propName) and
376-
this = headers.flow().(DataFlow::SourceNode).getAPropertyRead()
412+
this = headers.getAPropertyRead()
377413
|
378414
propName = "header" or
379415
propName = "headers"
380416
)
381417
or
382418
// `ctx.request.get(<name>)`
383-
this.asExpr().(MethodCallExpr).calls(e, "get")
419+
this.(DataFlow::MethodCallNode).calls(e, "get")
384420
)
385421
}
386422

@@ -427,9 +463,7 @@ module Koa {
427463
RouteHandler rh;
428464

429465
ResponseSendArgument() {
430-
exists(DataFlow::PropWrite pwn |
431-
pwn.writes(DataFlow::valueNode(rh.getAResponseOrContextExpr()), "body", this)
432-
)
466+
exists(DataFlow::PropWrite pwn | pwn.writes(rh.getAResponseOrContextNode(), "body", this))
433467
}
434468

435469
override RouteHandler getRouteHandler() { result = rh }
@@ -438,12 +472,10 @@ module Koa {
438472
/**
439473
* An invocation of the `redirect` method of an HTTP response object.
440474
*/
441-
private class RedirectInvocation extends HTTP::RedirectInvocation, DataFlow::MethodCallNode {
475+
private class RedirectInvocation extends HTTP::RedirectInvocation instanceof DataFlow::MethodCallNode {
442476
RouteHandler rh;
443477

444-
RedirectInvocation() {
445-
this.asExpr().(MethodCallExpr).calls(rh.getAResponseOrContextExpr(), "redirect")
446-
} // TODO: Improve this.
478+
RedirectInvocation() { super.calls(rh.getAResponseOrContextNode(), "redirect") }
447479

448480
override DataFlow::Node getUrlArgument() { result = this.getArgument(0) }
449481

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ module NodeJSLib {
4949
/**
5050
* Holds if `call` is an invocation of `http.createServer` or `https.createServer`.
5151
*/
52-
predicate isCreateServer(CallExpr call) {
53-
// TODO: DataFlow::Node
52+
predicate isCreateServer(DataFlow::CallNode call) {
5453
exists(string pkg, string fn |
5554
pkg = "http" and fn = "createServer"
5655
or
@@ -61,7 +60,7 @@ module NodeJSLib {
6160
or
6261
pkg = "http2" and fn = "createSecureServer"
6362
|
64-
call = DataFlow::moduleMember(pkg, fn).getAnInvocation().asExpr()
63+
call = DataFlow::moduleMember(pkg, fn).getAnInvocation()
6564
)
6665
}
6766

@@ -423,7 +422,7 @@ module NodeJSLib {
423422
* An expression that creates a new Node.js server.
424423
*/
425424
class ServerDefinition extends HTTP::Servers::StandardServerDefinition {
426-
ServerDefinition() { isCreateServer(this.asExpr()) }
425+
ServerDefinition() { isCreateServer(this) }
427426
}
428427

429428
/** An expression that is passed as `http.request({ auth: <expr> }, ...)`. */

‎javascript/ql/lib/semmle/javascript/security/dataflow/CleartextStorageCustomizations.qll

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ module CleartextStorage {
6161
/**
6262
* An expression set as a value of localStorage or sessionStorage.
6363
*/
64-
class WebStorageSink extends Sink {
65-
WebStorageSink() { this.asExpr() instanceof WebStorageWrite }
66-
}
64+
class WebStorageSink extends Sink instanceof WebStorageWrite { }
6765

6866
/**
6967
* An expression stored by AngularJS.

‎javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,9 @@ module ClientSideUrlRedirect {
104104
xss = true
105105
or
106106
// An assignment to `location`
107-
exists(Assignment assgn | isLocation(assgn.getTarget()) and astNode = assgn.getRhs()) and
107+
exists(Assignment assgn |
108+
isLocationNode(assgn.getTarget().flow()) and astNode = assgn.getRhs()
109+
) and
108110
xss = true
109111
or
110112
// An assignment to `location.href`, `location.protocol` or `location.hostname`

‎javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,18 +143,17 @@ class DomPropWriteNode extends Assignment {
143143
/**
144144
* A value written to web storage, like `localStorage` or `sessionStorage`.
145145
*/
146-
class WebStorageWrite extends Expr {
147-
// TODO: DataFlow::Node
146+
class WebStorageWrite extends DataFlow::Node {
148147
WebStorageWrite() {
149148
exists(DataFlow::SourceNode webStorage |
150149
webStorage = DataFlow::globalVarRef("localStorage") or
151150
webStorage = DataFlow::globalVarRef("sessionStorage")
152151
|
153152
// an assignment to `window.localStorage[someProp]`
154-
this = webStorage.getAPropertyWrite().getRhs().asExpr()
153+
this = webStorage.getAPropertyWrite().getRhs()
155154
or
156155
// an invocation of `window.localStorage.setItem`
157-
this = webStorage.getAMethodCall("setItem").getArgument(1).asExpr()
156+
this = webStorage.getAMethodCall("setItem").getArgument(1)
158157
)
159158
}
160159
}

‎javascript/ql/test/library-tests/frameworks/NodeJSLib/isCreateServer.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
import javascript
22

3-
query predicate test_isCreateServer(CallExpr e) { NodeJSLib::isCreateServer(e) }
3+
query predicate test_isCreateServer(DataFlow::CallNode e) { NodeJSLib::isCreateServer(e) }

‎javascript/ql/test/library-tests/frameworks/koa/ContextExpr.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import javascript
22

3-
query predicate test_ContextExpr(Koa::ContextExpr e, Koa::RouteHandler res) {
3+
query predicate test_ContextExpr(Koa::ContextNode e, Koa::RouteHandler res) {
44
res = e.getRouteHandler()
55
}

‎javascript/ql/test/library-tests/frameworks/koa/RouteHandler_getAContextExpr.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import semmle.javascript.frameworks.Express
22

3-
query predicate test_RouteHandler_getAContextExpr(Koa::RouteHandler rh, Expr res) {
4-
res = rh.getAContextExpr()
3+
query predicate test_RouteHandler_getAContextExpr(Koa::RouteHandler rh, DataFlow::Node res) {
4+
res = rh.getAContextNode()
55
}

‎javascript/ql/test/library-tests/frameworks/koa/tests.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ test_ResponseExpr
4747
| src/koa.js:18:3:18:14 | ctx.response | src/koa.js:10:10:28:1 | functio ... az');\\n} |
4848
| src/koa.js:44:2:44:13 | ctx.response | src/koa.js:30:10:45:1 | async c ... url);\\n} |
4949
test_RouteHandler_getAContextExpr
50+
| src/koa.js:7:1:7:22 | functio ... r1() {} | src/koa.js:7:1:7:0 | this |
51+
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:10:10:10:9 | this |
52+
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:10:28:10:30 | ctx |
5053
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:10:28:10:30 | ctx |
5154
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:11:3:11:6 | this |
5255
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:12:3:12:6 | this |
@@ -64,6 +67,7 @@ test_RouteHandler_getAContextExpr
6467
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:26:3:26:5 | ctx |
6568
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:27:3:27:5 | ctx |
6669
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:30:16:30:18 | ctx |
70+
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:30:16:30:18 | ctx |
6771
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:31:2:31:4 | ctx |
6872
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:32:2:32:4 | ctx |
6973
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:33:2:33:4 | ctx |
@@ -78,9 +82,11 @@ test_RouteHandler_getAContextExpr
7882
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:43:2:43:4 | ctx |
7983
| src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:44:2:44:4 | ctx |
8084
| src/koa.js:47:10:56:1 | async c ... .foo;\\n} | src/koa.js:47:16:47:18 | ctx |
85+
| src/koa.js:47:10:56:1 | async c ... .foo;\\n} | src/koa.js:47:16:47:18 | ctx |
8186
| src/koa.js:47:10:56:1 | async c ... .foo;\\n} | src/koa.js:48:16:48:18 | ctx |
8287
| src/koa.js:47:10:56:1 | async c ... .foo;\\n} | src/koa.js:51:14:51:16 | ctx |
8388
| src/koa.js:47:10:56:1 | async c ... .foo;\\n} | src/koa.js:54:16:54:18 | ctx |
89+
| src/koa.js:59:10:61:1 | functio ... .url;\\n} | src/koa.js:59:10:59:9 | this |
8490
| src/koa.js:59:10:61:1 | functio ... .url;\\n} | src/koa.js:60:2:60:5 | this |
8591
test_HeaderDefinition
8692
| src/koa.js:11:3:11:25 | this.se ... 1', '') | src/koa.js:10:10:28:1 | functio ... az');\\n} |
@@ -157,6 +163,9 @@ test_RouteHandler_getARequestExpr
157163
| src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:26:3:26:13 | ctx.request |
158164
| src/koa.js:59:10:61:1 | functio ... .url;\\n} | src/koa.js:60:2:60:13 | this.request |
159165
test_ContextExpr
166+
| src/koa.js:7:1:7:0 | this | src/koa.js:7:1:7:22 | functio ... r1() {} |
167+
| src/koa.js:10:10:10:9 | this | src/koa.js:10:10:28:1 | functio ... az');\\n} |
168+
| src/koa.js:10:28:10:30 | ctx | src/koa.js:10:10:28:1 | functio ... az');\\n} |
160169
| src/koa.js:10:28:10:30 | ctx | src/koa.js:10:10:28:1 | functio ... az');\\n} |
161170
| src/koa.js:11:3:11:6 | this | src/koa.js:10:10:28:1 | functio ... az');\\n} |
162171
| src/koa.js:12:3:12:6 | this | src/koa.js:10:10:28:1 | functio ... az');\\n} |
@@ -174,6 +183,7 @@ test_ContextExpr
174183
| src/koa.js:26:3:26:5 | ctx | src/koa.js:10:10:28:1 | functio ... az');\\n} |
175184
| src/koa.js:27:3:27:5 | ctx | src/koa.js:10:10:28:1 | functio ... az');\\n} |
176185
| src/koa.js:30:16:30:18 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
186+
| src/koa.js:30:16:30:18 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
177187
| src/koa.js:31:2:31:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
178188
| src/koa.js:32:2:32:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
179189
| src/koa.js:33:2:33:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
@@ -188,9 +198,11 @@ test_ContextExpr
188198
| src/koa.js:43:2:43:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
189199
| src/koa.js:44:2:44:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
190200
| src/koa.js:47:16:47:18 | ctx | src/koa.js:47:10:56:1 | async c ... .foo;\\n} |
201+
| src/koa.js:47:16:47:18 | ctx | src/koa.js:47:10:56:1 | async c ... .foo;\\n} |
191202
| src/koa.js:48:16:48:18 | ctx | src/koa.js:47:10:56:1 | async c ... .foo;\\n} |
192203
| src/koa.js:51:14:51:16 | ctx | src/koa.js:47:10:56:1 | async c ... .foo;\\n} |
193204
| src/koa.js:54:16:54:18 | ctx | src/koa.js:47:10:56:1 | async c ... .foo;\\n} |
205+
| src/koa.js:59:10:59:9 | this | src/koa.js:59:10:61:1 | functio ... .url;\\n} |
194206
| src/koa.js:60:2:60:5 | this | src/koa.js:59:10:61:1 | functio ... .url;\\n} |
195207
test_RedirectInvocation
196208
| src/koa.js:43:2:43:18 | ctx.redirect(url) | src/koa.js:43:15:43:17 | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |

0 commit comments

Comments
 (0)