Skip to content

Commit 291a576

Browse files
jacknojojacknojo
committed
Improve hapi coverage
1 parent 4239fee commit 291a576

7 files changed

Lines changed: 201 additions & 29 deletions

File tree

‎javascript/ql/lib/change-notes/2026-09-08-hapi-wrapped-routes.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* Improved Hapi route handler and request input tracking through custom route registration helpers and higher-order function wrappers.

‎javascript/ql/lib/semmle/javascript/dataflow/internal/FunctionWrapperSteps.qll‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,30 @@ private module Cached {
145145
}
146146

147147
import Cached
148+
149+
private DataFlow::SourceNode forwardedCalleeSource(
150+
DataFlow::CallNode call, DataFlow::TypeBackTracker t
151+
) {
152+
t.start() and
153+
result = call.getCalleeNode().getALocalSource()
154+
or
155+
exists(DataFlow::TypeBackTracker t2 | result = forwardedCalleeSource(call, t2).backtrack(t2, t))
156+
}
157+
158+
/** Data flow into a concrete function invoked through a forwarding wrapper. */
159+
private class FunctionWrapperCallStep extends DataFlow::SharedFlowStep {
160+
DataFlow::CallNode call;
161+
DataFlow::FunctionNode wrapped;
162+
163+
FunctionWrapperCallStep() {
164+
DataFlow::functionOneWayForwardingStep(wrapped,
165+
forwardedCalleeSource(call, DataFlow::TypeBackTracker::end()))
166+
}
167+
168+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
169+
exists(int index |
170+
pred = call.getArgument(index) and
171+
succ = wrapped.getParameter(index)
172+
)
173+
}
174+
}

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

Lines changed: 98 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import javascript
66
import semmle.javascript.frameworks.HTTP
7+
private import semmle.javascript.dataflow.internal.CallGraphs
78

89
module Hapi {
910
/**
@@ -116,17 +117,21 @@ module Hapi {
116117
this.(DataFlow::PropRead).accesses(request, "rawPayload")
117118
or
118119
exists(DataFlow::PropRead payload |
119-
// `request.payload.name`
120+
// `request.payload.name`, or `request.payload` when the object is forwarded.
120121
payload.accesses(request, "payload") and
121-
this.(DataFlow::PropRead).accesses(payload, _)
122+
if exists(payload.getAPropertyRead())
123+
then this = payload.getAPropertyRead()
124+
else this = payload
122125
)
123126
)
124127
or
125128
kind = "parameter" and
126-
exists(DataFlow::PropRead query |
127-
// `request.query.name`
128-
query.accesses(request, ["query", "params"]) and
129-
this.(DataFlow::PropRead).accesses(query, _)
129+
exists(DataFlow::PropRead parameter |
130+
// `request.query.name` / `request.params.name`, or the object when it is forwarded.
131+
parameter.accesses(request, ["query", "params"]) and
132+
if exists(parameter.getAPropertyRead())
133+
then this = parameter.getAPropertyRead()
134+
else this = parameter
130135
)
131136
or
132137
exists(DataFlow::PropRead url |
@@ -199,30 +204,10 @@ module Hapi {
199204
*/
200205
class RouteSetup extends DataFlow::MethodCallNode, Http::Servers::StandardRouteSetup {
201206
ServerDefinition server;
202-
DataFlow::Node handler;
203207

204208
RouteSetup() {
205209
server.ref().getAMethodCall() = this and
206-
(
207-
// server.route({ handler: fun })
208-
this.getMethodName() = "route" and
209-
this.getOptionArgument(0, "handler") = handler
210-
or
211-
// server.ext('/', fun)
212-
this.getMethodName() = "ext" and
213-
handler = this.getArgument(1)
214-
or
215-
// server.route([{ handler(request){}])
216-
this.getMethodName() = "route" and
217-
handler =
218-
this.getArgument(0)
219-
.getALocalSource()
220-
.(DataFlow::ArrayCreationNode)
221-
.getAnElement()
222-
.getALocalSource()
223-
.getAPropertySource("handler")
224-
.getAFunctionValue()
225-
)
210+
this.getMethodName() = ["route", "ext"]
226211
}
227212

228213
override DataFlow::SourceNode getARouteHandler() {
@@ -233,11 +218,45 @@ module Hapi {
233218
t.start() and
234219
result = this.getRouteHandler().getALocalSource()
235220
or
236-
exists(DataFlow::TypeBackTracker t2 | result = this.getARouteHandler(t2).backtrack(t2, t))
221+
this.getMethodName() = "route" and
222+
t.isInProp("handler") and
223+
result = this.getArgument(0).getALocalSource()
224+
or
225+
exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ |
226+
succ = this.getARouteHandler(t2)
227+
|
228+
result = succ.backtrack(t2, t)
229+
or
230+
Http::routeHandlerStep(result, succ) and
231+
t = t2
232+
or
233+
DataFlow::SharedFlowStep::storeStep(result.getALocalUse(), succ,
234+
DataFlow::PseudoProperties::arrayElement()) and
235+
t = t2.continue()
236+
)
237237
}
238238

239239
pragma[noinline]
240-
private DataFlow::Node getRouteHandler() { result = handler }
240+
private DataFlow::Node getRouteHandler() {
241+
// server.route({ handler: fun })
242+
this.getMethodName() = "route" and
243+
this.getOptionArgument(0, "handler") = result
244+
or
245+
// server.ext('/', fun)
246+
this.getMethodName() = "ext" and
247+
result = this.getArgument(1)
248+
or
249+
// server.route([{ handler(request){}])
250+
this.getMethodName() = "route" and
251+
result =
252+
this.getArgument(0)
253+
.getALocalSource()
254+
.(DataFlow::ArrayCreationNode)
255+
.getAnElement()
256+
.getALocalSource()
257+
.getAPropertySource("handler")
258+
.getAFunctionValue()
259+
}
241260

242261
override DataFlow::Node getServer() { result = server }
243262
}
@@ -263,6 +282,56 @@ module Hapi {
263282
}
264283
}
265284

285+
private DataFlow::SourceNode routeDefinitionRef(
286+
DataFlow::ObjectLiteralNode definition, DataFlow::TypeTracker t
287+
) {
288+
t.start() and
289+
result = definition
290+
or
291+
exists(DataFlow::TypeTracker t2 | result = routeDefinitionRef(definition, t2).track(t2, t))
292+
}
293+
294+
private predicate handlerRegistration(
295+
DataFlow::FunctionNode handler, DataFlow::ObjectLiteralNode definition
296+
) {
297+
exists(
298+
DataFlow::CallNode registration, DataFlow::FunctionNode registrar,
299+
DataFlow::ParameterNode handlerParameter, DataFlow::SourceNode handlerRef, int index
300+
|
301+
registration.getACallee() = registrar.getFunction() and
302+
handlerParameter = registrar.getParameter(index) and
303+
handlerParameter.flowsTo(definition.getAPropertyWrite("handler").getRhs()) and
304+
(
305+
handlerRef = handler
306+
or
307+
handlerRef = CallGraph::callgraphStep(handler, DataFlow::TypeTracker::end())
308+
) and
309+
handlerRef.flowsTo(registration.getArgument(index))
310+
)
311+
}
312+
313+
/** Data flow through handlers stored in route definitions by registration helpers. */
314+
private class RegisteredHandlerCallStep extends DataFlow::SharedFlowStep {
315+
DataFlow::CallNode call;
316+
DataFlow::FunctionNode handler;
317+
318+
RegisteredHandlerCallStep() {
319+
exists(DataFlow::ObjectLiteralNode definition, DataFlow::PropRead handlerRead |
320+
handlerRegistration(handler, definition) and
321+
handlerRead.getPropertyName() = "handler" and
322+
routeDefinitionRef(definition, DataFlow::TypeTracker::end()).flowsTo(handlerRead.getBase()) and
323+
call.getCalleeNode() = handlerRead
324+
)
325+
}
326+
327+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
328+
exists(int index |
329+
pred = call.getArgument(index) and
330+
succ = handler.getParameter(index)
331+
)
332+
}
333+
}
334+
266335
/**
267336
* A function that looks like a Hapi route handler and flows to a route setup.
268337
*/

‎javascript/ql/test/library-tests/frameworks/hapi/WrappedRouteFlow.qll‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import javascript
2+
3+
module WrappedRouteConfig implements DataFlow::ConfigSig {
4+
predicate isSource(DataFlow::Node source) { source instanceof Http::RequestInputAccess }
5+
6+
predicate isSink(DataFlow::Node sink) {
7+
sink = DataFlow::globalVarRef("sink").getACall().getArgument(0)
8+
}
9+
}
10+
11+
module WrappedRouteTaint = TaintTracking::Global<WrappedRouteConfig>;
12+
13+
query predicate test_WrappedRouteFlow(DataFlow::Node source, DataFlow::Node sink) {
14+
WrappedRouteTaint::flow(source, sink)
15+
}

‎javascript/ql/test/library-tests/frameworks/hapi/src/wrapped-route.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
const Hapi = require("hapi");
2+
3+
const endpoints = [];
4+
5+
function endpoint(handler) {
6+
endpoints.push({ handler });
7+
}
8+
9+
function routeConfig(handler) {
10+
return {
11+
handler: async function (request, h) {
12+
return handler(request.query);
13+
},
14+
};
15+
}
16+
17+
function createCached(fn) {
18+
return async function (...args) {
19+
return fn(...args);
20+
};
21+
}
22+
23+
const cached = createCached(function (filter) {
24+
sink(filter);
25+
});
26+
27+
class Routes {
28+
get(query) {
29+
return cached(query.filter);
30+
}
31+
}
32+
33+
endpoint(Routes.prototype.get);
34+
35+
function register(server, instance) {
36+
for (const definition of endpoints) {
37+
const wrapped = async (query) => definition.handler.call(instance, query);
38+
server.route(routeConfig(wrapped));
39+
}
40+
}
41+
42+
register(new Hapi.Server(), new Routes());

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ test_RouteSetup
1414
| src/hapihapi.js:17:1:18:2 | server2 ... dler\\n}) |
1515
| src/hapihapi.js:29:1:29:20 | server2.route(route) |
1616
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) |
17+
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) |
1718
test_RequestExpr
1819
| src/hapi.js:13:32:13:38 | request | src/hapi.js:13:14:15:5 | functio ... n\\n } |
1920
| src/hapi.js:13:32:13:38 | request | src/hapi.js:13:14:15:5 | functio ... n\\n } |
@@ -56,6 +57,9 @@ test_RequestExpr
5657
| src/hapihapi.js:25:3:25:9 | request | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
5758
| src/hapihapi.js:26:3:26:9 | request | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
5859
| src/hapihapi.js:34:22:34:24 | req | src/hapihapi.js:34:12:34:30 | function (req, h){} |
60+
| src/wrapped-route.js:11:30:11:36 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
61+
| src/wrapped-route.js:11:30:11:36 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
62+
| src/wrapped-route.js:12:22:12:28 | request | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
5963
test_HeaderAccess
6064
| src/hapi.js:25:3:25:21 | request.headers.baz | baz |
6165
| src/hapiglue.js:27:3:27:21 | request.headers.baz | baz |
@@ -80,6 +84,7 @@ test_RouteHandler
8084
| src/hapihapi.js:17:30:18:1 | functio ... ndler\\n} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
8185
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
8286
| src/hapihapi.js:34:12:34:30 | function (req, h){} | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
87+
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
8388
test_HeaderDefinition
8489
| src/hapi.js:14:9:14:46 | request ... 1', '') | src/hapi.js:13:14:15:5 | functio ... n\\n } |
8590
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | src/hapiglue.js:13:14:15:5 | functio ... n\\n } |
@@ -93,6 +98,7 @@ test_ServerDefinition
9398
| src/hapiglue.js:44:45:44:51 | server_ |
9499
| src/hapihapi.js:1:15:1:50 | new (re ... erver() |
95100
| src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
101+
| src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
96102
test_RequestInputAccess
97103
| src/hapi.js:21:3:21:20 | request.rawPayload | body | src/hapi.js:20:1:27:1 | functio ... oken;\\n} |
98104
| src/hapi.js:22:3:22:21 | request.payload.foo | body | src/hapi.js:20:1:27:1 | functio ... oken;\\n} |
@@ -114,6 +120,7 @@ test_RequestInputAccess
114120
| src/hapihapi.js:24:3:24:18 | request.url.path | url | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
115121
| src/hapihapi.js:25:3:25:21 | request.headers.baz | header | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
116122
| src/hapihapi.js:26:3:26:21 | request.state.token | cookie | src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} |
123+
| src/wrapped-route.js:12:22:12:34 | request.query | parameter | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
117124
test_RouteSetup_getServer
118125
| src/hapi.js:7:1:9:2 | server2 ... ler1\\n}) | src/hapi.js:4:15:4:31 | new Hapi.Server() |
119126
| src/hapi.js:12:1:15:7 | server2 ... }}) | src/hapi.js:4:15:4:31 | new Hapi.Server() |
@@ -130,6 +137,7 @@ test_RouteSetup_getServer
130137
| src/hapihapi.js:17:1:18:2 | server2 ... dler\\n}) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
131138
| src/hapihapi.js:29:1:29:20 | server2.route(route) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
132139
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:4:15:4:31 | new Hapi.Server() |
140+
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) | src/wrapped-route.js:42:10:42:26 | new Hapi.Server() |
133141
test_HeaderDefinition_defines
134142
| src/hapi.js:14:9:14:46 | request ... 1', '') | header1 | |
135143
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | header1 | |
@@ -156,6 +164,7 @@ test_RouteSetup_getARouteHandler
156164
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:33:1:35:1 | return of function getHandler |
157165
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:34:12:34:30 | function (req, h){} |
158166
| src/hapihapi.js:36:1:36:38 | server2 ... ler()}) | src/hapihapi.js:36:25:36:36 | getHandler() |
167+
| src/wrapped-route.js:38:5:38:38 | server. ... apped)) | src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } |
159168
test_RouteHandler_getARequestExpr
160169
| src/hapi.js:13:14:15:5 | functio ... n\\n } | src/hapi.js:13:32:13:38 | request |
161170
| src/hapi.js:13:14:15:5 | functio ... n\\n } | src/hapi.js:13:32:13:38 | request |
@@ -198,6 +207,9 @@ test_RouteHandler_getARequestExpr
198207
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:25:3:25:9 | request |
199208
| src/hapihapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapihapi.js:26:3:26:9 | request |
200209
| src/hapihapi.js:34:12:34:30 | function (req, h){} | src/hapihapi.js:34:22:34:24 | req |
210+
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:11:30:11:36 | request |
211+
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:11:30:11:36 | request |
212+
| src/wrapped-route.js:11:14:13:5 | async f ... ;\\n } | src/wrapped-route.js:12:22:12:28 | request |
201213
test_HeaderDefinition_getAHeaderName
202214
| src/hapi.js:14:9:14:46 | request ... 1', '') | header1 |
203215
| src/hapiglue.js:14:9:14:46 | request ... 1', '') | header1 |
@@ -206,3 +218,5 @@ test_RouteHandler_getAResponseHeader
206218
| src/hapi.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapi.js:14:9:14:46 | request ... 1', '') |
207219
| src/hapiglue.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapiglue.js:14:9:14:46 | request ... 1', '') |
208220
| src/hapihapi.js:13:14:15:5 | functio ... n\\n } | header1 | src/hapihapi.js:14:9:14:46 | request ... 1', '') |
221+
test_WrappedRouteFlow
222+
| src/wrapped-route.js:12:22:12:34 | request.query | src/wrapped-route.js:24:8:24:13 | filter |

‎javascript/ql/test/library-tests/frameworks/hapi/tests.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ import RouteSetup_getARouteHandler
1212
import RouteHandler
1313
import RequestExpr
1414
import RouteHandler_getARequestExpr
15+
import WrappedRouteFlow

0 commit comments

Comments
 (0)