Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added support for the `fastify` `addHook` method.
39 changes: 37 additions & 2 deletions javascript/ql/lib/semmle/javascript/Routing.qll
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ module Routing {
predicate mayResumeDispatch() {
this.getLastChild().mayResumeDispatch()
or
isInMiddlewareSetup(this)
or
exists(this.(RouteHandler).getAContinuationInvocation())
or
// Leaf nodes that aren't functions are assumed to invoke their continuation
Expand All @@ -155,6 +157,8 @@ module Routing {
predicate definitelyResumesDispatch() {
this.getLastChild().definitelyResumesDispatch()
or
isInMiddlewareSetup(this)
or
exists(this.(RouteHandler).getAContinuationInvocation())
or
this instanceof MkRouter
Expand Down Expand Up @@ -325,6 +329,19 @@ module Routing {
DataFlow::Node getValueImplicitlyStoredInAccessPath(int n, string path) { none() }
}

/**
* Holds if `node` is installed at a route handler that is declared to be a middleware setup,
* and is therefore assume to resume dispatch.
*/
private predicate isInMiddlewareSetup(Node node) {
exists(RouteSetup::Range range |
node = getRouteSetupNode(range) and
range.isMiddlewareSetup()
)
or
isInMiddlewareSetup(node.getParent())
}

/** Holds if `pred` and `succ` are adjacent siblings and `succ` is installed after `pred`. */
private predicate areSiblings(Node pred, Node succ) {
exists(ValueNode::Range base, int n |
Expand Down Expand Up @@ -612,6 +629,20 @@ module Routing {
* Holds if this route setup targets `router` and occurs at the given `cfgNode`.
*/
abstract predicate isInstalledAt(Router::Range router, ControlFlowNode cfgNode);

/**
* Holds if this is a middleware setup, meaning dispatch will resume after the
* route handlers in this route setup have completed (usually meaning that they have returned a promise, which has resolved).
*
* This should only be overridden when the route setup itself determines whether subsequent
* route handlers are invoked afterwards.
* - For Express-like libraries, the route _handler_ determines whether to resume dispatch,
* based on whether the `next` callback is invoked. For such libraries, do not override `isMiddlewareSetup`.
* - For Fastify-like libraries, the route _setup_ determines whether to resume dispatch.
* For example, `.addHook()` will resume dispatch whereas `.get()` will not. `isMiddlewareSetup()` should thus
* hold for `.addHook()` but not for `.get()` calls.
*/
predicate isMiddlewareSetup() { none() }
}

/**
Expand Down Expand Up @@ -892,10 +923,14 @@ module Routing {
* based on `Node::Range::getValueAtAccessPath`.
*/
private DataFlow::Node getAnAccessPathRhs(Node base, int n, string path) {
// Assigned in the body of a route handler function, whi
// Assigned in the body of a route handler function, which is a middleware
exists(RouteHandler handler | base = handler |
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path) and
exists(handler.getAContinuationInvocation())
(
exists(handler.getAContinuationInvocation())
or
isInMiddlewareSetup(handler)
)
)
or
// Implicit assignment contributed by framework model
Expand Down
16 changes: 13 additions & 3 deletions javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ module Fastify {

RouteSetup() {
this = server(server).getAMethodCall(methodName) and
methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch"]
methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch", "addHook"]
}

override DataFlow::SourceNode getARouteHandler() {
Expand All @@ -164,13 +164,19 @@ module Fastify {

private class ShorthandRoutingTreeSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup
{
ShorthandRoutingTreeSetup() { not this.getMethodName() = "route" }
ShorthandRoutingTreeSetup() { not this.getMethodName() = ["route", "addHook"] }

override string getRelativePath() { result = this.getArgument(0).getStringValue() }

override Http::RequestMethodName getHttpMethod() { result = this.getMethodName().toUpperCase() }
}

private class AddHookRouteSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup {
AddHookRouteSetup() { this.getMethodName() = "addHook" }

override predicate isMiddlewareSetup() { any() }
}

/** Gets the name of the `n`th handler function that can be installed a route setup, in order of execution. */
private string getNthHandlerName(int n) {
result =
Expand Down Expand Up @@ -322,7 +328,11 @@ module Fastify {
ResponseSendArgument() {
this = rh.getAResponseSource().ref().getAMethodCall("send").getArgument(0)
or
this = rh.(DataFlow::FunctionNode).getAReturn()
exists(RouteSetup setup |
rh = setup.getARouteHandler() and
this = rh.(DataFlow::FunctionNode).getAReturn() and
setup.getMethodName() != "addHook"
)
}

override RouteHandler getRouteHandler() { result = rh }
Expand Down
Loading