Skip to content

Commit 0d2c679

Browse files
authored
Merge pull request #22544 from dtorres-fgf/fastify-chainable-server-methods
JS: recognize Fastify servers reached through chainable server methods
2 parents c364999 + f5fa010 commit 0d2c679

6 files changed

Lines changed: 92 additions & 0 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Fastify servers reached through a chainable configuration method, such as `fastify().withTypeProvider<T>()` or `fastify().setValidatorCompiler(...)`, are now recognized as the same server instance. Routes registered on such an instance are now attributed to their server, which may add results for queries such as `js/missing-rate-limiting` where routes were previously not recognized at all, and remove false positives where a globally registered plugin guards them.

javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,27 @@ module Fastify {
2121
StandardServerDefinition() { this = DataFlow::moduleImport("fastify").getAnInvocation() }
2222
}
2323

24+
/**
25+
* Gets the name of a chainable Fastify configuration method, that is, a method that
26+
* configures the server instance and returns that same instance, so that a call to it
27+
* still refers to the server.
28+
*
29+
* Plugin, hook and route registration (`register`, `addHook`, `onClose`, and the
30+
* shorthand route methods) returns the server as well, but is deliberately excluded
31+
* here, because it already has a meaning in the routing model for Fastify. The
32+
* lifecycle methods `after` and `ready` are excluded too: `after` returns the server
33+
* only when it is given a callback, and `ready` never does.
34+
*/
35+
private string chainableConfigMethodName() {
36+
result =
37+
[
38+
"withTypeProvider", "addSchema", "addHttpMethod", "addContentTypeParser", "decorate",
39+
"decorateRequest", "decorateReply", "setValidatorCompiler", "setSerializerCompiler",
40+
"setSchemaController", "setReplySerializer", "setSchemaErrorFormatter", "setErrorHandler",
41+
"setNotFoundHandler", "setGenReqId", "setChildLoggerFactory"
42+
]
43+
}
44+
2445
/** Gets a data flow node referring to a fastify server. */
2546
private DataFlow::SourceNode server(DataFlow::SourceNode creation, DataFlow::TypeTracker t) {
2647
t.start() and
@@ -31,6 +52,11 @@ module Fastify {
3152
t.start() and
3253
result = pluginCallback(creation).(DataFlow::FunctionNode).getParameter(0)
3354
or
55+
// server.withTypeProvider<T>(), server.setValidatorCompiler(...), and friends return
56+
// the server itself, so the result of such a call still refers to it.
57+
t.start() and
58+
result = server(creation).getAMethodCall(chainableConfigMethodName())
59+
or
3460
exists(DataFlow::TypeTracker t2 | result = server(creation, t2).track(t2, t))
3561
}
3662

javascript/ql/test/library-tests/frameworks/fastify/src/fastify.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,13 @@ fastifyWithObjects4.post(
9090
request.params;
9191
}
9292
);
93+
94+
// the server is reached through a chainable configuration method, which returns the
95+
// same instance
96+
var fastifyChained = require("fastify")().withTypeProvider();
97+
fastifyChained.get(
98+
"/",
99+
/* handler */ (request, reply) => {
100+
reply.send({ hello: "world" }); // response
101+
}
102+
);

javascript/ql/test/library-tests/frameworks/fastify/tests.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ test_RouteSetup
77
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) |
88
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) |
99
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) |
10+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) |
1011
test_HeaderAccess
1112
| src/fastify.js:39:5:39:24 | request.headers.name | name |
1213
test_RouteHandler
@@ -25,6 +26,7 @@ test_RouteHandler
2526
| src/fastify.js:65:17:69:3 | functio ... ms;\\n } | src/fastify.js:61:27:61:46 | require("fastify")() |
2627
| src/fastify.js:76:17:80:3 | functio ... ms;\\n } | src/fastify.js:72:27:72:46 | require("fastify")() |
2728
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:83:27:83:46 | require("fastify")() |
29+
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:96:22:96:41 | require("fastify")() |
2830
test_HeaderDefinition
2931
| src/fastify.js:42:5:42:33 | reply.h ... value") | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
3032
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
@@ -34,6 +36,7 @@ test_ServerDefinition
3436
| src/fastify.js:61:27:61:46 | require("fastify")() |
3537
| src/fastify.js:72:27:72:46 | require("fastify")() |
3638
| src/fastify.js:83:27:83:46 | require("fastify")() |
39+
| src/fastify.js:96:22:96:41 | require("fastify")() |
3740
test_RedirectInvocation
3841
| src/fastify.js:44:5:44:29 | reply.r ... e, url) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
3942
test_RequestInputAccess
@@ -57,6 +60,7 @@ test_ResponseSendArgument
5760
| src/fastify.js:6:12:6:29 | { hello: "world" } | src/fastify.js:5:17:7:3 | async ( ... nse\\n } |
5861
| src/fastify.js:27:16:27:33 | { hello: "world" } | src/fastify.js:26:17:28:3 | (reques ... nse\\n } |
5962
| src/fastify.js:45:16:45:22 | payload | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
63+
| src/fastify.js:100:16:100:33 | { hello: "world" } | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
6064
test_RouteSetup_getServer
6165
| src/fastify.js:3:1:8:1 | fastify ... e\\n }\\n) | src/fastify.js:1:15:1:34 | require("fastify")() |
6266
| src/fastify.js:10:1:21:2 | fastify ... > {}\\n}) | src/fastify.js:1:15:1:34 | require("fastify")() |
@@ -66,6 +70,7 @@ test_RouteSetup_getServer
6670
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:61:27:61:46 | require("fastify")() |
6771
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:72:27:72:46 | require("fastify")() |
6872
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:83:27:83:46 | require("fastify")() |
73+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:96:22:96:41 | require("fastify")() |
6974
test_HeaderDefinition_defines
7075
| src/fastify.js:42:5:42:33 | reply.h ... value") | name | value |
7176
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name | value |
@@ -85,6 +90,7 @@ test_RouteSetup_getARouteHandler
8590
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:65:17:69:3 | functio ... ms;\\n } |
8691
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:76:17:80:3 | functio ... ms;\\n } |
8792
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:87:17:91:3 | functio ... ms;\\n } |
93+
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
8894
test_RouteHandler_getARequestExpr
8995
| src/fastify.js:5:17:7:3 | async ( ... nse\\n } | src/fastify.js:5:24:5:30 | request |
9096
| src/fastify.js:13:28:13:55 | (reques ... ) => {} | src/fastify.js:13:29:13:35 | request |
@@ -122,6 +128,7 @@ test_RouteHandler_getARequestExpr
122128
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:88:5:88:11 | request |
123129
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:89:5:89:11 | request |
124130
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:90:5:90:11 | request |
131+
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:99:18:99:24 | request |
125132
test_HeaderDefinition_getAHeaderName
126133
| src/fastify.js:42:5:42:33 | reply.h ... value") | name |
127134
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name |

javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,5 @@
1111
| tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
1212
| tst.js:111:28:111:44 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
1313
| tst.js:116:39:116:55 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
14+
| tst.js:130:35:130:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
15+
| tst.js:161:35:161:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |

javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,46 @@ const fastifyApp3 = require('fastify')();
116116
fastifyApp3.get('/before-rate-limit', expensiveHandler1); // $ Alert
117117
fastifyApp3.register(require('@fastify/rate-limit'));
118118
fastifyApp3.get('/after-rate-limit', expensiveHandler1);
119+
120+
// the server instance is reached through a chainable configuration method, which
121+
// returns the same instance
122+
const fastifyApp4 = require('fastify')().withTypeProvider();
123+
124+
fastifyApp4.register(require('@fastify/rate-limit'));
125+
fastifyApp4.get('/after-rate-limit', expensiveHandler1);
126+
127+
// same, but with no rate limiter registered at all, so the route is genuinely unguarded
128+
const fastifyApp5 = require('fastify')().withTypeProvider();
129+
130+
fastifyApp5.get('/no-rate-limit', expensiveHandler1); // $ Alert
131+
132+
// several configuration methods chained together
133+
const fastifyApp6 = require('fastify')()
134+
.withTypeProvider()
135+
.setValidatorCompiler(compiler)
136+
.addContentTypeParser('application/json', parser)
137+
.decorate('answer', 42);
138+
139+
fastifyApp6.register(require('@fastify/rate-limit'));
140+
fastifyApp6.get('/after-rate-limit', expensiveHandler1);
141+
142+
// the chained instance is returned from a factory function, so reaching it requires
143+
// tracking the value across the call rather than only through local references
144+
function makeFastifyApp() {
145+
return require('fastify')().withTypeProvider();
146+
}
147+
148+
const fastifyApp7 = makeFastifyApp();
149+
150+
fastifyApp7.register(require('@fastify/rate-limit'));
151+
fastifyApp7.get('/after-rate-limit', expensiveHandler1);
152+
153+
// same, from a separate factory so that the server above does not share its creation
154+
// site, and no rate limiter is registered on it
155+
function makeUnguardedFastifyApp() {
156+
return require('fastify')().withTypeProvider();
157+
}
158+
159+
const fastifyApp8 = makeUnguardedFastifyApp();
160+
161+
fastifyApp8.get('/no-rate-limit', expensiveHandler1); // $ Alert

0 commit comments

Comments
 (0)