Skip to content
Draft
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: fix
---
* Improved Hapi route handler and request input tracking through custom route registration helpers and higher-order function wrappers.
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,30 @@ private module Cached {
}

import Cached

private DataFlow::SourceNode forwardedCalleeSource(
DataFlow::CallNode call, DataFlow::TypeBackTracker t
) {
t.start() and
result = call.getCalleeNode().getALocalSource()
or
exists(DataFlow::TypeBackTracker t2 | result = forwardedCalleeSource(call, t2).backtrack(t2, t))
}

/** Data flow into a concrete function invoked through a forwarding wrapper. */
private class FunctionWrapperCallStep extends DataFlow::SharedFlowStep {
DataFlow::CallNode call;
DataFlow::FunctionNode wrapped;

FunctionWrapperCallStep() {
DataFlow::functionOneWayForwardingStep(wrapped,
forwardedCalleeSource(call, DataFlow::TypeBackTracker::end()))
}

override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists(int index |
pred = call.getArgument(index) and
succ = wrapped.getParameter(index)
)
}
}
127 changes: 98 additions & 29 deletions javascript/ql/lib/semmle/javascript/frameworks/Hapi.qll
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import javascript
import semmle.javascript.frameworks.HTTP
private import semmle.javascript.dataflow.internal.CallGraphs

module Hapi {
/**
Expand Down Expand Up @@ -116,17 +117,21 @@ module Hapi {
this.(DataFlow::PropRead).accesses(request, "rawPayload")
or
exists(DataFlow::PropRead payload |
// `request.payload.name`
// `request.payload.name`, or `request.payload` when the object is forwarded.
payload.accesses(request, "payload") and
this.(DataFlow::PropRead).accesses(payload, _)
if exists(payload.getAPropertyRead())
then this = payload.getAPropertyRead()
else this = payload
)
)
or
kind = "parameter" and
exists(DataFlow::PropRead query |
// `request.query.name`
query.accesses(request, ["query", "params"]) and
this.(DataFlow::PropRead).accesses(query, _)
exists(DataFlow::PropRead parameter |
// `request.query.name` / `request.params.name`, or the object when it is forwarded.
parameter.accesses(request, ["query", "params"]) and
if exists(parameter.getAPropertyRead())
then this = parameter.getAPropertyRead()
else this = parameter
)
or
exists(DataFlow::PropRead url |
Expand Down Expand Up @@ -199,30 +204,10 @@ module Hapi {
*/
class RouteSetup extends DataFlow::MethodCallNode, Http::Servers::StandardRouteSetup {
ServerDefinition server;
DataFlow::Node handler;

RouteSetup() {
server.ref().getAMethodCall() = this and
(
// server.route({ handler: fun })
this.getMethodName() = "route" and
this.getOptionArgument(0, "handler") = handler
or
// server.ext('/', fun)
this.getMethodName() = "ext" and
handler = this.getArgument(1)
or
// server.route([{ handler(request){}])
this.getMethodName() = "route" and
handler =
this.getArgument(0)
.getALocalSource()
.(DataFlow::ArrayCreationNode)
.getAnElement()
.getALocalSource()
.getAPropertySource("handler")
.getAFunctionValue()
)
this.getMethodName() = ["route", "ext"]
}

override DataFlow::SourceNode getARouteHandler() {
Expand All @@ -233,11 +218,45 @@ module Hapi {
t.start() and
result = this.getRouteHandler().getALocalSource()
or
exists(DataFlow::TypeBackTracker t2 | result = this.getARouteHandler(t2).backtrack(t2, t))
this.getMethodName() = "route" and
t.isInProp("handler") and
result = this.getArgument(0).getALocalSource()
or
exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ |
succ = this.getARouteHandler(t2)
|
result = succ.backtrack(t2, t)
or
Http::routeHandlerStep(result, succ) and
t = t2
or
DataFlow::SharedFlowStep::storeStep(result.getALocalUse(), succ,
DataFlow::PseudoProperties::arrayElement()) and
t = t2.continue()
)
}

pragma[noinline]
private DataFlow::Node getRouteHandler() { result = handler }
private DataFlow::Node getRouteHandler() {
// server.route({ handler: fun })
this.getMethodName() = "route" and
this.getOptionArgument(0, "handler") = result
or
// server.ext('/', fun)
this.getMethodName() = "ext" and
result = this.getArgument(1)
or
// server.route([{ handler(request){}])
this.getMethodName() = "route" and
result =
this.getArgument(0)
.getALocalSource()
.(DataFlow::ArrayCreationNode)
.getAnElement()
.getALocalSource()
.getAPropertySource("handler")
.getAFunctionValue()
}

override DataFlow::Node getServer() { result = server }
}
Expand All @@ -263,6 +282,56 @@ module Hapi {
}
}

private DataFlow::SourceNode routeDefinitionRef(
DataFlow::ObjectLiteralNode definition, DataFlow::TypeTracker t
) {
t.start() and
result = definition
or
exists(DataFlow::TypeTracker t2 | result = routeDefinitionRef(definition, t2).track(t2, t))
}

private predicate handlerRegistration(
DataFlow::FunctionNode handler, DataFlow::ObjectLiteralNode definition
) {
exists(
DataFlow::CallNode registration, DataFlow::FunctionNode registrar,
DataFlow::ParameterNode handlerParameter, DataFlow::SourceNode handlerRef, int index
|
registration.getACallee() = registrar.getFunction() and
handlerParameter = registrar.getParameter(index) and
handlerParameter.flowsTo(definition.getAPropertyWrite("handler").getRhs()) and
(
handlerRef = handler
or
handlerRef = CallGraph::callgraphStep(handler, DataFlow::TypeTracker::end())
) and
handlerRef.flowsTo(registration.getArgument(index))
)
}

/** Data flow through handlers stored in route definitions by registration helpers. */
private class RegisteredHandlerCallStep extends DataFlow::SharedFlowStep {
DataFlow::CallNode call;
DataFlow::FunctionNode handler;

RegisteredHandlerCallStep() {
exists(DataFlow::ObjectLiteralNode definition, DataFlow::PropRead handlerRead |
handlerRegistration(handler, definition) and
handlerRead.getPropertyName() = "handler" and
routeDefinitionRef(definition, DataFlow::TypeTracker::end()).flowsTo(handlerRead.getBase()) and
call.getCalleeNode() = handlerRead
)
}

override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
exists(int index |
pred = call.getArgument(index) and
succ = handler.getParameter(index)
)
}
}

/**
* A function that looks like a Hapi route handler and flows to a route setup.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import javascript

module WrappedRouteConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Http::RequestInputAccess }

predicate isSink(DataFlow::Node sink) {
sink = DataFlow::globalVarRef("sink").getACall().getArgument(0)
}
}

module WrappedRouteTaint = TaintTracking::Global<WrappedRouteConfig>;

query predicate test_WrappedRouteFlow(DataFlow::Node source, DataFlow::Node sink) {
WrappedRouteTaint::flow(source, sink)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
const Hapi = require("hapi");

const endpoints = [];

function endpoint(handler) {
endpoints.push({ handler });
}

function routeConfig(handler) {
return {
handler: async function (request, h) {
return handler(request.query);
},
};
}

function createCached(fn) {
return async function (...args) {
return fn(...args);
};
}

const cached = createCached(function (filter) {
sink(filter);
});

class Routes {
get(query) {
return cached(query.filter);
}
}

endpoint(Routes.prototype.get);

function register(server, instance) {
for (const definition of endpoints) {
const wrapped = async (query) => definition.handler.call(instance, query);
server.route(routeConfig(wrapped));
}
}

register(new Hapi.Server(), new Routes());
14 changes: 14 additions & 0 deletions javascript/ql/test/library-tests/frameworks/hapi/tests.expected
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ test_RouteSetup
| src/hapihapi.js:17:1:18:2 | server2 ... dler\\n}) |
| src/hapihapi.js:29:1:29:20 | server2.route(route) |
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) |
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) |
test_RequestExpr
| src/hapi.js:13:32:13:38 | request | src/hapi.js:13:14:15:5 | functio ... n\\n } |
| src/hapi.js:13:32:13:38 | request | src/hapi.js:13:14:15:5 | functio ... n\\n } |
Expand Down Expand Up @@ -56,6 +57,9 @@ test_RequestExpr
| src/hapihapi.js:25:3:25:9 | request | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/hapihapi.js:26:3:26:9 | request | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/hapihapi.js:34:22:34:24 | req | src/hapihapi.js:34:12:34:30 | function (req, h){} |
| src/wrapped-route.js:11:30:11:36 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
| src/wrapped-route.js:11:30:11:36 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
| src/wrapped-route.js:12:22:12:28 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
test_HeaderAccess
| src/hapi.js:25:3:25:21 | request.headers.baz | baz |
| src/hapiglue.js:27:3:27:21 | request.headers.baz | baz |
Expand All @@ -80,6 +84,7 @@ test_RouteHandler
| src/hapihapi.js:17:30:18:1 | functio ... ndler\\n} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/hapihapi.js:34:12:34:30 | function (req, h){} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
test_HeaderDefinition
| src/hapi.js:14:9:14:46 | request ... 1', '') | src/hapi.js:13:14:15:5 | functio ... n\\n } |
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | src/hapiglue.js:13:14:15:5 | functio ... n\\n } |
Expand All @@ -93,6 +98,7 @@ test_ServerDefinition
| src/hapiglue.js:44:45:44:51 | server_ |
| src/hapihapi.js:1:15:1:50 | new (re ... erver() |
| src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
test_RequestInputAccess
| src/hapi.js:21:3:21:20 | request.rawPayload | body | src/hapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/hapi.js:22:3:22:21 | request.payload.foo | body | src/hapi.js:20:1:27:1 | functio ... oken;\\n} |
Expand All @@ -114,6 +120,7 @@ test_RequestInputAccess
| src/hapihapi.js:24:3:24:18 | request.url.path | url | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/hapihapi.js:25:3:25:21 | request.headers.baz | header | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/hapihapi.js:26:3:26:21 | request.state.token | cookie | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
| src/wrapped-route.js:12:22:12:34 | request.query | parameter | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
test_RouteSetup_getServer
| src/hapi.js:7:1:9:2 | server2 ... ler1\\n}) | src/hapi.js:4:15:4:31 | new Hapi.Server() |
| src/hapi.js:12:1:15:7 | server2 ... }}) | src/hapi.js:4:15:4:31 | new Hapi.Server() |
Expand All @@ -130,6 +137,7 @@ test_RouteSetup_getServer
| src/hapihapi.js:17:1:18:2 | server2 ... dler\\n}) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/hapihapi.js:29:1:29:20 | server2.route(route) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) | src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
test_HeaderDefinition_defines
| src/hapi.js:14:9:14:46 | request ... 1', '') | header1 | |
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | header1 | |
Expand All @@ -156,6 +164,7 @@ test_RouteSetup_getARouteHandler
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:33:1:35:1 | return of function getHandler |
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:34:12:34:30 | function (req, h){} |
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:36:25:36:36 | getHandler() |
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
test_RouteHandler_getARequestExpr
| src/hapi.js:13:14:15:5 | functio ... n\\n } | src/hapi.js:13:32:13:38 | request |
| src/hapi.js:13:14:15:5 | functio ... n\\n } | src/hapi.js:13:32:13:38 | request |
Expand Down Expand Up @@ -198,6 +207,9 @@ test_RouteHandler_getARequestExpr
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:25:3:25:9 | request |
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:26:3:26:9 | request |
| src/hapihapi.js:34:12:34:30 | function (req, h){} | src/hapihapi.js:34:22:34:24 | req |
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:11:30:11:36 | request |
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:11:30:11:36 | request |
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:12:22:12:28 | request |
test_HeaderDefinition_getAHeaderName
| src/hapi.js:14:9:14:46 | request ... 1', '') | header1 |
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | header1 |
Expand All @@ -206,3 +218,5 @@ test_RouteHandler_getAResponseHeader
| src/hapi.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapi.js:14:9:14:46 | request ... 1', '') |
| src/hapiglue.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapiglue.js:14:9:14:46 | request ... 1', '') |
| src/hapihapi.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapihapi.js:14:9:14:46 | request ... 1', '') |
test_WrappedRouteFlow
| src/wrapped-route.js:12:22:12:34 | request.query | src/wrapped-route.js:24:8:24:13 | filter |
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ import RouteSetup_getARouteHandler
import RouteHandler
import RequestExpr
import RouteHandler_getARequestExpr
import WrappedRouteFlow
Loading