Skip to content
Open
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
---
* 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.
26 changes: 26 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,27 @@ module Fastify {
StandardServerDefinition() { this = DataFlow::moduleImport("fastify").getAnInvocation() }
}

/**
* Gets the name of a chainable Fastify configuration method, that is, a method that
* configures the server instance and returns that same instance, so that a call to it
* still refers to the server.
*
* Plugin, hook and route registration (`register`, `addHook`, `onClose`, and the
* shorthand route methods) returns the server as well, but is deliberately excluded
* here, because it already has a meaning in the routing model for Fastify. The
* lifecycle methods `after` and `ready` are excluded too: `after` returns the server
* only when it is given a callback, and `ready` never does.
*/
private string chainableConfigMethodName() {
result =
[
"withTypeProvider", "addSchema", "addHttpMethod", "addContentTypeParser", "decorate",
"decorateRequest", "decorateReply", "setValidatorCompiler", "setSerializerCompiler",
"setSchemaController", "setReplySerializer", "setSchemaErrorFormatter", "setErrorHandler",
"setNotFoundHandler", "setGenReqId", "setChildLoggerFactory"
]
}

/** Gets a data flow node referring to a fastify server. */
private DataFlow::SourceNode server(DataFlow::SourceNode creation, DataFlow::TypeTracker t) {
t.start() and
Expand All @@ -31,6 +52,11 @@ module Fastify {
t.start() and
result = pluginCallback(creation).(DataFlow::FunctionNode).getParameter(0)
or
// server.withTypeProvider<T>(), server.setValidatorCompiler(...), and friends return
// the server itself, so the result of such a call still refers to it.
t.start() and
result = server(creation).getAMethodCall(chainableConfigMethodName())
or
exists(DataFlow::TypeTracker t2 | result = server(creation, t2).track(t2, t))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,13 @@ fastifyWithObjects4.post(
request.params;
}
);

// the server is reached through a chainable configuration method, which returns the
// same instance
var fastifyChained = require("fastify")().withTypeProvider();
fastifyChained.get(
"/",
/* handler */ (request, reply) => {
reply.send({ hello: "world" }); // response
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ test_RouteSetup
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) |
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) |
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) |
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) |
test_HeaderAccess
| src/fastify.js:39:5:39:24 | request.headers.name | name |
test_RouteHandler
Expand All @@ -25,6 +26,7 @@ test_RouteHandler
| src/fastify.js:65:17:69:3 | functio ... ms;\\n } | src/fastify.js:61:27:61:46 | require("fastify")() |
| src/fastify.js:76:17:80:3 | functio ... ms;\\n } | src/fastify.js:72:27:72:46 | require("fastify")() |
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:83:27:83:46 | require("fastify")() |
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:96:22:96:41 | require("fastify")() |
test_HeaderDefinition
| src/fastify.js:42:5:42:33 | reply.h ... value") | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
Expand All @@ -34,6 +36,7 @@ test_ServerDefinition
| src/fastify.js:61:27:61:46 | require("fastify")() |
| src/fastify.js:72:27:72:46 | require("fastify")() |
| src/fastify.js:83:27:83:46 | require("fastify")() |
| src/fastify.js:96:22:96:41 | require("fastify")() |
test_RedirectInvocation
| src/fastify.js:44:5:44:29 | reply.r ... e, url) | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
test_RequestInputAccess
Expand All @@ -57,6 +60,7 @@ test_ResponseSendArgument
| src/fastify.js:6:12:6:29 | { hello: "world" } | src/fastify.js:5:17:7:3 | async ( ... nse\\n } |
| src/fastify.js:27:16:27:33 | { hello: "world" } | src/fastify.js:26:17:28:3 | (reques ... nse\\n } |
| src/fastify.js:45:16:45:22 | payload | src/fastify.js:34:17:46:3 | functio ... eam\\n } |
| src/fastify.js:100:16:100:33 | { hello: "world" } | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
test_RouteSetup_getServer
| src/fastify.js:3:1:8:1 | fastify ... e\\n }\\n) | src/fastify.js:1:15:1:34 | require("fastify")() |
| src/fastify.js:10:1:21:2 | fastify ... > {}\\n}) | src/fastify.js:1:15:1:34 | require("fastify")() |
Expand All @@ -66,6 +70,7 @@ test_RouteSetup_getServer
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:61:27:61:46 | require("fastify")() |
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:72:27:72:46 | require("fastify")() |
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:83:27:83:46 | require("fastify")() |
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:96:22:96:41 | require("fastify")() |
test_HeaderDefinition_defines
| src/fastify.js:42:5:42:33 | reply.h ... value") | name | value |
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name | value |
Expand All @@ -85,6 +90,7 @@ test_RouteSetup_getARouteHandler
| src/fastify.js:63:1:70:1 | fastify ... ;\\n }\\n) | src/fastify.js:65:17:69:3 | functio ... ms;\\n } |
| src/fastify.js:74:1:81:1 | fastify ... ;\\n }\\n) | src/fastify.js:76:17:80:3 | functio ... ms;\\n } |
| src/fastify.js:85:1:92:1 | fastify ... ;\\n }\\n) | src/fastify.js:87:17:91:3 | functio ... ms;\\n } |
| src/fastify.js:97:1:102:1 | fastify ... e\\n }\\n) | src/fastify.js:99:17:101:3 | (reques ... nse\\n } |
test_RouteHandler_getARequestExpr
| src/fastify.js:5:17:7:3 | async ( ... nse\\n } | src/fastify.js:5:24:5:30 | request |
| src/fastify.js:13:28:13:55 | (reques ... ) => {} | src/fastify.js:13:29:13:35 | request |
Expand Down Expand Up @@ -122,6 +128,7 @@ test_RouteHandler_getARequestExpr
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:88:5:88:11 | request |
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:89:5:89:11 | request |
| src/fastify.js:87:17:91:3 | functio ... ms;\\n } | src/fastify.js:90:5:90:11 | request |
| src/fastify.js:99:17:101:3 | (reques ... nse\\n } | src/fastify.js:99:18:99:24 | request |
test_HeaderDefinition_getAHeaderName
| src/fastify.js:42:5:42:33 | reply.h ... value") | name |
| src/fastify.js:43:5:43:36 | reply.h ... lue" }) | name |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@
| tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
| tst.js:111:28:111:44 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
| tst.js:116:39:116:55 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
| tst.js:130:35:130:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
| tst.js:161:35:161:51 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization |
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,46 @@ const fastifyApp3 = require('fastify')();
fastifyApp3.get('/before-rate-limit', expensiveHandler1); // $ Alert
fastifyApp3.register(require('@fastify/rate-limit'));
fastifyApp3.get('/after-rate-limit', expensiveHandler1);

// the server instance is reached through a chainable configuration method, which
// returns the same instance
const fastifyApp4 = require('fastify')().withTypeProvider();

fastifyApp4.register(require('@fastify/rate-limit'));
fastifyApp4.get('/after-rate-limit', expensiveHandler1);

// same, but with no rate limiter registered at all, so the route is genuinely unguarded
const fastifyApp5 = require('fastify')().withTypeProvider();

fastifyApp5.get('/no-rate-limit', expensiveHandler1); // $ Alert

// several configuration methods chained together
const fastifyApp6 = require('fastify')()
.withTypeProvider()
.setValidatorCompiler(compiler)
.addContentTypeParser('application/json', parser)
.decorate('answer', 42);

fastifyApp6.register(require('@fastify/rate-limit'));
fastifyApp6.get('/after-rate-limit', expensiveHandler1);

// the chained instance is returned from a factory function, so reaching it requires
// tracking the value across the call rather than only through local references
function makeFastifyApp() {
return require('fastify')().withTypeProvider();
}

const fastifyApp7 = makeFastifyApp();

fastifyApp7.register(require('@fastify/rate-limit'));
fastifyApp7.get('/after-rate-limit', expensiveHandler1);

// same, from a separate factory so that the server above does not share its creation
// site, and no rate limiter is registered on it
function makeUnguardedFastifyApp() {
return require('fastify')().withTypeProvider();
}

const fastifyApp8 = makeUnguardedFastifyApp();

fastifyApp8.get('/no-rate-limit', expensiveHandler1); // $ Alert
Loading