Skip to content

Commit 8266b08

Browse files
erik-krogherik-krogh
committed
update the predicates on Express::RouteHandler to use dataflow nodes
1 parent 4cfbf15 commit 8266b08

File tree

13 files changed

+69
-68
lines changed

13 files changed

+69
-68
lines changed

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

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,29 +30,31 @@ module Connect {
3030
*
3131
* `kind` is one of: "error", "request", "response", "next".
3232
*/
33-
abstract Parameter getRouteHandlerParameter(string kind);
33+
abstract DataFlow::ParameterNode getRouteHandlerParameter(string kind);
3434

3535
/**
3636
* Gets the parameter of the route handler that contains the request object.
3737
*/
38-
override Parameter getRequestParameter() { result = getRouteHandlerParameter("request") }
38+
override DataFlow::ParameterNode getRequestParameter() {
39+
result = getRouteHandlerParameter("request")
40+
}
3941

4042
/**
4143
* Gets the parameter of the route handler that contains the response object.
4244
*/
43-
override Parameter getResponseParameter() { result = getRouteHandlerParameter("response") }
45+
override DataFlow::ParameterNode getResponseParameter() {
46+
result = getRouteHandlerParameter("response")
47+
}
4448
}
4549

4650
/**
4751
* A Connect route handler installed by a route setup.
4852
*/
49-
class StandardRouteHandler extends RouteHandler {
50-
override Function astNode;
51-
53+
class StandardRouteHandler extends RouteHandler, DataFlow::FunctionNode {
5254
StandardRouteHandler() { this = any(RouteSetup setup).getARouteHandler() }
5355

54-
override Parameter getRouteHandlerParameter(string kind) {
55-
result = getRouteHandlerParameter(astNode, kind)
56+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
57+
result = getRouteHandlerParameter(this, kind)
5658
}
5759
}
5860

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ module ConnectExpressShared {
5252
/**
5353
* Holds if `function` appears to match the given signature based on parameter naming.
5454
*/
55-
private predicate matchesSignature(Function function, RouteHandlerSignature sig) {
55+
private predicate matchesSignature(DataFlow::FunctionNode function, RouteHandlerSignature sig) {
5656
function.getNumParameter() = sig.getArity() and
5757
function.getParameter(sig.getParameterIndex("request")).getName() = ["req", "request"] and
5858
function.getParameter(sig.getParameterIndex("response")).getName() = ["res", "response"] and
@@ -71,8 +71,8 @@ module ConnectExpressShared {
7171
* so the caller should restrict the function accordingly.
7272
*/
7373
pragma[inline]
74-
private Parameter getRouteHandlerParameter(
75-
Function routeHandler, RouteHandlerSignature sig, string kind
74+
private DataFlow::ParameterNode getRouteHandlerParameter(
75+
DataFlow::FunctionNode routeHandler, RouteHandlerSignature sig, string kind
7676
) {
7777
result = routeHandler.getParameter(sig.getParameterIndex(kind))
7878
}
@@ -83,7 +83,9 @@ module ConnectExpressShared {
8383
* `kind` is one of: "error", "request", "response", "next".
8484
*/
8585
pragma[inline]
86-
Parameter getRouteParameterHandlerParameter(Function routeHandler, string kind) {
86+
DataFlow::ParameterNode getRouteParameterHandlerParameter(
87+
DataFlow::FunctionNode routeHandler, string kind
88+
) {
8789
result =
8890
getRouteHandlerParameter(routeHandler, RouteHandlerSignature::requestResponseNextParameter(),
8991
kind)
@@ -95,7 +97,7 @@ module ConnectExpressShared {
9597
* `kind` is one of: "error", "request", "response", "next".
9698
*/
9799
pragma[inline]
98-
Parameter getRouteHandlerParameter(Function routeHandler, string kind) {
100+
DataFlow::ParameterNode getRouteHandlerParameter(DataFlow::FunctionNode routeHandler, string kind) {
99101
if routeHandler.getNumParameter() = 4
100102
then
101103
// For arity 4 there is ambiguity between (err, req, res, next) and (req, res, next, param)
@@ -115,7 +117,7 @@ module ConnectExpressShared {
115117
*/
116118
class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate {
117119
RouteHandlerCandidate() {
118-
matchesSignature(astNode, _) and
120+
matchesSignature(this, _) and
119121
not (
120122
// heuristic: not a class method (the server invokes this with a function call)
121123
astNode = any(MethodDefinition def).getBody()

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

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -286,14 +286,12 @@ module Express {
286286
* The callback given to passport in PassportRouteSetup.
287287
*/
288288
private class PassportRouteHandler extends RouteHandler, HTTP::Servers::StandardRouteHandler,
289-
DataFlow::ValueNode {
290-
override Function astNode;
291-
289+
DataFlow::FunctionNode {
292290
PassportRouteHandler() { this = any(PassportRouteSetup setup).getARouteHandler() }
293291

294-
override Parameter getRouteHandlerParameter(string kind) {
292+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
295293
kind = "request" and
296-
result = astNode.getParameter(0)
294+
result = this.getParameter(0)
297295
}
298296
}
299297

@@ -478,40 +476,41 @@ module Express {
478476
*
479477
* `kind` is one of: "error", "request", "response", "next", or "parameter".
480478
*/
481-
abstract Parameter getRouteHandlerParameter(string kind); // TODO: DataFlow::ParameterNode
479+
abstract DataFlow::ParameterNode getRouteHandlerParameter(string kind);
482480

483481
/**
484482
* Gets the parameter of the route handler that contains the request object.
485483
*/
486-
Parameter getRequestParameter() { result = this.getRouteHandlerParameter("request") } // TODO: DataFlow::ParameterNode
484+
DataFlow::ParameterNode getRequestParameter() {
485+
result = this.getRouteHandlerParameter("request")
486+
}
487487

488488
/**
489489
* Gets the parameter of the route handler that contains the response object.
490490
*/
491-
Parameter getResponseParameter() { result = this.getRouteHandlerParameter("response") } // TODO: DataFlow::ParameterNode
491+
DataFlow::ParameterNode getResponseParameter() {
492+
result = this.getRouteHandlerParameter("response")
493+
}
492494

493495
/**
494496
* Gets a request body access of this handler.
495497
*/
496-
Expr getARequestBodyAccess() {
497-
result.(PropAccess).accesses(this.getARequestNode().asExpr(), "body")
498-
} // TODO: DataFlow::Node
498+
DataFlow::PropRead getARequestBodyAccess() { result.accesses(this.getARequestNode(), "body") }
499499
}
500500

501501
/**
502502
* An Express route handler installed by a route setup.
503503
*/
504504
class StandardRouteHandler extends RouteHandler, HTTP::Servers::StandardRouteHandler,
505-
DataFlow::ValueNode {
506-
override Function astNode;
505+
DataFlow::FunctionNode {
507506
RouteSetup routeSetup;
508507

509508
StandardRouteHandler() { this = routeSetup.getARouteHandler() }
510509

511-
override Parameter getRouteHandlerParameter(string kind) {
510+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
512511
if routeSetup.isParameterHandler()
513-
then result = getRouteParameterHandlerParameter(astNode, kind)
514-
else result = getRouteHandlerParameter(astNode, kind)
512+
then result = getRouteParameterHandlerParameter(this, kind)
513+
else result = getRouteHandlerParameter(this, kind)
515514
}
516515
}
517516

@@ -540,7 +539,7 @@ module Express {
540539
RouteHandler rh;
541540

542541
ExplicitResponseSource() {
543-
this = DataFlow::parameterNode(rh.getResponseParameter())
542+
this = rh.getResponseParameter()
544543
or
545544
isChainableResponseMethodCall(rh, this.asExpr())
546545
}
@@ -570,7 +569,7 @@ module Express {
570569
private class ExplicitRequestSource extends RequestSource {
571570
RouteHandler rh;
572571

573-
ExplicitRequestSource() { this = DataFlow::parameterNode(rh.getRequestParameter()) }
572+
ExplicitRequestSource() { this = rh.getRequestParameter() }
574573

575574
/**
576575
* Gets the route handler that handles this request.
@@ -637,7 +636,7 @@ module Express {
637636

638637
ParamHandlerInputAccess() {
639638
exists(RouteSetup setup | rh = setup.getARouteHandler() |
640-
this = DataFlow::parameterNode(rh.getRouteHandlerParameter("parameter"))
639+
this = rh.getRouteHandlerParameter("parameter")
641640
)
642641
}
643642

@@ -778,7 +777,8 @@ module Express {
778777
* An access to the HTTP request body.
779778
*/
780779
class RequestBodyAccess extends Expr {
781-
RequestBodyAccess() { any(RouteHandler h).getARequestBodyAccess() = this }
780+
// TODO: DataFlow::Node
781+
RequestBodyAccess() { any(RouteHandler h).getARequestBodyAccess().asExpr() = this }
782782
}
783783

784784
abstract private class HeaderDefinition extends HTTP::Servers::StandardHeaderDefinition {
@@ -1049,10 +1049,10 @@ module Express {
10491049

10501050
TrackedRouteHandlerCandidateWithSetup() { this = routeSetup.getARouteHandler() }
10511051

1052-
override Parameter getRouteHandlerParameter(string kind) {
1052+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
10531053
if routeSetup.isParameterHandler()
1054-
then result = getRouteParameterHandlerParameter(astNode, kind)
1055-
else result = getRouteHandlerParameter(astNode, kind)
1054+
then result = getRouteParameterHandlerParameter(this, kind)
1055+
else result = getRouteHandlerParameter(this, kind)
10561056
}
10571057
}
10581058

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,15 +216,13 @@ module Firebase {
216216
* A function used as a route handler.
217217
*/
218218
private class RouteHandler extends Express::RouteHandler, HTTP::Servers::StandardRouteHandler,
219-
DataFlow::ValueNode {
220-
override Function astNode;
221-
219+
DataFlow::FunctionNode {
222220
RouteHandler() { this = any(RouteSetup setup).getARouteHandler() }
223221

224-
override Parameter getRouteHandlerParameter(string kind) {
225-
kind = "request" and result = astNode.getParameter(0)
222+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
223+
kind = "request" and result = this.getParameter(0)
226224
or
227-
kind = "response" and result = astNode.getParameter(1)
225+
kind = "response" and result = this.getParameter(1)
228226
}
229227
}
230228
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,16 +83,12 @@ private module HttpProxy {
8383
)
8484
}
8585

86-
override Parameter getRequestParameter() {
87-
exists(int req | routeHandlingEventHandler(event, req, _) |
88-
result = getFunction().getParameter(req)
89-
)
86+
override DataFlow::ParameterNode getRequestParameter() {
87+
exists(int req | routeHandlingEventHandler(event, req, _) | result = getParameter(req))
9088
}
9189

92-
override Parameter getResponseParameter() {
93-
exists(int res | routeHandlingEventHandler(event, _, res) |
94-
result = getFunction().getParameter(res)
95-
)
90+
override DataFlow::ParameterNode getResponseParameter() {
91+
exists(int res | routeHandlingEventHandler(event, _, res) | result = getParameter(res))
9692
}
9793
}
9894
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ private module LiveServer {
2222
class RouteHandler extends Connect::RouteHandler, DataFlow::FunctionNode {
2323
RouteHandler() { this = any(RouteSetup setup).getARouteHandler() }
2424

25-
override Parameter getRouteHandlerParameter(string kind) {
26-
result = ConnectExpressShared::getRouteHandlerParameter(astNode, kind)
25+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
26+
result = ConnectExpressShared::getRouteHandlerParameter(this, kind)
2727
}
2828
}
2929

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,10 +230,10 @@ module NextJS {
230230
)
231231
}
232232

233-
override Parameter getRouteHandlerParameter(string kind) {
234-
kind = "request" and result = this.getFunction().getParameter(0)
233+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
234+
kind = "request" and result = this.getParameter(0)
235235
or
236-
kind = "response" and result = this.getFunction().getParameter(1)
236+
kind = "response" and result = this.getParameter(1)
237237
}
238238
}
239239

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,12 @@ module NodeJSLib {
114114
/**
115115
* Gets the parameter of the route handler that contains the request object.
116116
*/
117-
Parameter getRequestParameter() { result = this.getFunction().getParameter(0) }
117+
DataFlow::ParameterNode getRequestParameter() { result = this.getParameter(0) }
118118

119119
/**
120120
* Gets the parameter of the route handler that contains the response object.
121121
*/
122-
Parameter getResponseParameter() { result = this.getFunction().getParameter(1) }
122+
DataFlow::ParameterNode getResponseParameter() { result = this.getParameter(1) }
123123
}
124124

125125
/**
@@ -141,7 +141,7 @@ module NodeJSLib {
141141
private class StandardResponseSource extends ResponseSource {
142142
RouteHandler rh;
143143

144-
StandardResponseSource() { this = DataFlow::parameterNode(rh.getResponseParameter()) }
144+
StandardResponseSource() { this = rh.getResponseParameter() }
145145

146146
/**
147147
* Gets the route handler that provides this response.
@@ -161,7 +161,7 @@ module NodeJSLib {
161161
private class StandardRequestSource extends RequestSource {
162162
RouteHandler rh;
163163

164-
StandardRequestSource() { this = DataFlow::parameterNode(rh.getRequestParameter()) }
164+
StandardRequestSource() { this = rh.getRequestParameter() }
165165

166166
/**
167167
* Gets the route handler that handles this request.

‎javascript/ql/lib/semmle/javascript/heuristics/AdditionalRouteHandlers.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ private class PromotedExpressCandidate extends Express::RouteHandler,
2929
HTTP::Servers::StandardRouteHandler {
3030
PromotedExpressCandidate() { this instanceof ConnectExpressShared::RouteHandlerCandidate }
3131

32-
override Parameter getRouteHandlerParameter(string kind) {
33-
result = ConnectExpressShared::getRouteHandlerParameter(getAstNode(), kind)
32+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
33+
result = ConnectExpressShared::getRouteHandlerParameter(this, kind)
3434
}
3535
}
3636

@@ -41,7 +41,7 @@ private class PromotedConnectCandidate extends Connect::RouteHandler,
4141
HTTP::Servers::StandardRouteHandler {
4242
PromotedConnectCandidate() { this instanceof ConnectExpressShared::RouteHandlerCandidate }
4343

44-
override Parameter getRouteHandlerParameter(string kind) {
45-
result = ConnectExpressShared::getRouteHandlerParameter(getAstNode(), kind)
44+
override DataFlow::ParameterNode getRouteHandlerParameter(string kind) {
45+
result = ConnectExpressShared::getRouteHandlerParameter(this, kind)
4646
}
4747
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ class RateLimiterFlexibleRateLimiter extends DataFlow::FunctionNode {
199199
rateLimiterClassName.matches("RateLimiter%") and
200200
rateLimiterClass = API::moduleImport("rate-limiter-flexible").getMember(rateLimiterClassName) and
201201
rateLimiterConsume = rateLimiterClass.getInstance().getMember("consume") and
202-
request.getParameter() = getRouteHandlerParameter(this.getFunction(), "request") and
202+
request = getRouteHandlerParameter(this, "request") and
203203
request.getAPropertyRead().flowsTo(rateLimiterConsume.getAParameter().asSink())
204204
)
205205
}

0 commit comments

Comments
 (0)