From 6d617663662a2dd4a152dff9c2357865508b0248 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Apr 2025 14:50:35 +0200 Subject: [PATCH 1/4] Added test case for `fastify.all` --- .../query-tests/Security/CWE-094/CodeInjection/fastify.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js index e1cba0d277ca..d538898f947a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js @@ -101,3 +101,10 @@ fastify.get('/flow-through-reply', async (request, reply) => { } return { result: null }; }); + +fastify.all('/eval', async (request, reply) => { + const userInput = request.query.code; // $ MISSING: Source[js/code-injection] + const result = eval(userInput); // $ MISSING: Alert[js/code-injection] + const replyResult = eval(reply.locals.nestedCode); // $ MISSING: Alert[js/code-injection] + return { method: request.method, result }; +}); From 71f1b82a56245c63d447f5fc704add27ee02996f Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Apr 2025 14:54:09 +0200 Subject: [PATCH 2/4] Added support for `fastify.all` --- .../ql/lib/semmle/javascript/frameworks/Fastify.qll | 7 +++++-- .../CWE-094/CodeInjection/CodeInjection.expected | 13 +++++++++++++ .../HeuristicSourceCodeInjection.expected | 9 +++++++++ .../Security/CWE-094/CodeInjection/fastify.js | 6 +++--- 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll b/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll index cf8dd76b75e3..4b53292e1482 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll @@ -138,7 +138,8 @@ module Fastify { RouteSetup() { this = server(server).getAMethodCall(methodName) and - methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch", "addHook"] + methodName = + ["route", "get", "head", "post", "put", "delete", "options", "patch", "addHook", "all"] } override DataFlow::SourceNode getARouteHandler() { @@ -168,7 +169,9 @@ module Fastify { override string getRelativePath() { result = this.getArgument(0).getStringValue() } - override Http::RequestMethodName getHttpMethod() { result = this.getMethodName().toUpperCase() } + override Http::RequestMethodName getHttpMethod() { + if this.getMethodName() = "all" then any() else result = this.getMethodName().toUpperCase() + } } private class AddHookRouteSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup { diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected index 9ad04af3a2c5..4d54adb27249 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected @@ -57,6 +57,10 @@ | fastify.js:84:30:84:43 | reply.userCode | fastify.js:79:20:79:42 | request ... plyCode | fastify.js:84:30:84:43 | reply.userCode | This code execution depends on a $@. | fastify.js:79:20:79:42 | request ... plyCode | user-provided value | | fastify.js:99:30:99:52 | reply.l ... tedCode | fastify.js:94:29:94:41 | request.query | fastify.js:99:30:99:52 | reply.l ... tedCode | This code execution depends on a $@. | fastify.js:94:29:94:41 | request.query | user-provided value | | fastify.js:99:30:99:52 | reply.l ... tedCode | fastify.js:94:29:94:51 | request ... plyCode | fastify.js:99:30:99:52 | reply.l ... tedCode | This code execution depends on a $@. | fastify.js:94:29:94:51 | request ... plyCode | user-provided value | +| fastify.js:107:23:107:31 | userInput | fastify.js:106:21:106:33 | request.query | fastify.js:107:23:107:31 | userInput | This code execution depends on a $@. | fastify.js:106:21:106:33 | request.query | user-provided value | +| fastify.js:107:23:107:31 | userInput | fastify.js:106:21:106:38 | request.query.code | fastify.js:107:23:107:31 | userInput | This code execution depends on a $@. | fastify.js:106:21:106:38 | request.query.code | user-provided value | +| fastify.js:108:28:108:50 | reply.l ... tedCode | fastify.js:94:29:94:41 | request.query | fastify.js:108:28:108:50 | reply.l ... tedCode | This code execution depends on a $@. | fastify.js:94:29:94:41 | request.query | user-provided value | +| fastify.js:108:28:108:50 | reply.l ... tedCode | fastify.js:94:29:94:51 | request ... plyCode | fastify.js:108:28:108:50 | reply.l ... tedCode | This code execution depends on a $@. | fastify.js:94:29:94:51 | request ... plyCode | user-provided value | | module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code | module.js:9:16:9:29 | req.query.code | This code execution depends on a $@. | module.js:9:16:9:29 | req.query.code | user-provided value | | module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code | module.js:11:17:11:30 | req.query.code | This code execution depends on a $@. | module.js:11:17:11:30 | req.query.code | user-provided value | | react-native.js:8:32:8:38 | tainted | react-native.js:7:17:7:33 | req.param("code") | react-native.js:8:32:8:38 | tainted | This code execution depends on a $@. | react-native.js:7:17:7:33 | req.param("code") | user-provided value | @@ -145,6 +149,10 @@ edges | fastify.js:79:20:79:42 | request ... plyCode | fastify.js:84:30:84:43 | reply.userCode | provenance | | | fastify.js:94:29:94:41 | request.query | fastify.js:94:29:94:51 | request ... plyCode | provenance | | | fastify.js:94:29:94:51 | request ... plyCode | fastify.js:99:30:99:52 | reply.l ... tedCode | provenance | | +| fastify.js:94:29:94:51 | request ... plyCode | fastify.js:108:28:108:50 | reply.l ... tedCode | provenance | | +| fastify.js:106:9:106:38 | userInput | fastify.js:107:23:107:31 | userInput | provenance | | +| fastify.js:106:21:106:33 | request.query | fastify.js:106:9:106:38 | userInput | provenance | | +| fastify.js:106:21:106:38 | request.query.code | fastify.js:106:9:106:38 | userInput | provenance | | | react-native.js:7:7:7:33 | tainted | react-native.js:8:32:8:38 | tainted | provenance | | | react-native.js:7:7:7:33 | tainted | react-native.js:10:23:10:29 | tainted | provenance | | | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | provenance | | @@ -268,6 +276,11 @@ nodes | fastify.js:94:29:94:41 | request.query | semmle.label | request.query | | fastify.js:94:29:94:51 | request ... plyCode | semmle.label | request ... plyCode | | fastify.js:99:30:99:52 | reply.l ... tedCode | semmle.label | reply.l ... tedCode | +| fastify.js:106:9:106:38 | userInput | semmle.label | userInput | +| fastify.js:106:21:106:33 | request.query | semmle.label | request.query | +| fastify.js:106:21:106:38 | request.query.code | semmle.label | request.query.code | +| fastify.js:107:23:107:31 | userInput | semmle.label | userInput | +| fastify.js:108:28:108:50 | reply.l ... tedCode | semmle.label | reply.l ... tedCode | | module.js:9:16:9:29 | req.query.code | semmle.label | req.query.code | | module.js:11:17:11:30 | req.query.code | semmle.label | req.query.code | | react-native.js:7:7:7:33 | tainted | semmle.label | tainted | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected index aa23d7a6d5a1..a1c8354ecf71 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected @@ -51,6 +51,10 @@ edges | fastify.js:79:20:79:42 | request ... plyCode | fastify.js:84:30:84:43 | reply.userCode | provenance | | | fastify.js:94:29:94:41 | request.query | fastify.js:94:29:94:51 | request ... plyCode | provenance | | | fastify.js:94:29:94:51 | request ... plyCode | fastify.js:99:30:99:52 | reply.l ... tedCode | provenance | | +| fastify.js:94:29:94:51 | request ... plyCode | fastify.js:108:28:108:50 | reply.l ... tedCode | provenance | | +| fastify.js:106:9:106:38 | userInput | fastify.js:107:23:107:31 | userInput | provenance | | +| fastify.js:106:21:106:33 | request.query | fastify.js:106:9:106:38 | userInput | provenance | | +| fastify.js:106:21:106:38 | request.query.code | fastify.js:106:9:106:38 | userInput | provenance | | | react-native.js:7:7:7:33 | tainted | react-native.js:8:32:8:38 | tainted | provenance | | | react-native.js:7:7:7:33 | tainted | react-native.js:10:23:10:29 | tainted | provenance | | | react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | provenance | | @@ -176,6 +180,11 @@ nodes | fastify.js:94:29:94:41 | request.query | semmle.label | request.query | | fastify.js:94:29:94:51 | request ... plyCode | semmle.label | request ... plyCode | | fastify.js:99:30:99:52 | reply.l ... tedCode | semmle.label | reply.l ... tedCode | +| fastify.js:106:9:106:38 | userInput | semmle.label | userInput | +| fastify.js:106:21:106:33 | request.query | semmle.label | request.query | +| fastify.js:106:21:106:38 | request.query.code | semmle.label | request.query.code | +| fastify.js:107:23:107:31 | userInput | semmle.label | userInput | +| fastify.js:108:28:108:50 | reply.l ... tedCode | semmle.label | reply.l ... tedCode | | module.js:9:16:9:29 | req.query.code | semmle.label | req.query.code | | module.js:11:17:11:30 | req.query.code | semmle.label | req.query.code | | react-native.js:7:7:7:33 | tainted | semmle.label | tainted | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js index d538898f947a..05dd3f6eb463 100644 --- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js +++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js @@ -103,8 +103,8 @@ fastify.get('/flow-through-reply', async (request, reply) => { }); fastify.all('/eval', async (request, reply) => { - const userInput = request.query.code; // $ MISSING: Source[js/code-injection] - const result = eval(userInput); // $ MISSING: Alert[js/code-injection] - const replyResult = eval(reply.locals.nestedCode); // $ MISSING: Alert[js/code-injection] + const userInput = request.query.code; // $ Source[js/code-injection] + const result = eval(userInput); // $ Alert[js/code-injection] + const replyResult = eval(reply.locals.nestedCode); // $ Alert[js/code-injection] return { method: request.method, result }; }); From 9624a413e4b3a464ec68a9073516572819fc535f Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 30 Apr 2025 14:57:00 +0200 Subject: [PATCH 3/4] Added change note --- javascript/ql/lib/change-notes/2025-04-30-fastify-all.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-04-30-fastify-all.md diff --git a/javascript/ql/lib/change-notes/2025-04-30-fastify-all.md b/javascript/ql/lib/change-notes/2025-04-30-fastify-all.md new file mode 100644 index 000000000000..a49092f6ba41 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-04-30-fastify-all.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Enhanced modeling of the [fastify](https://www.npmjs.com/package/fastify) framework to support the `all` route handler method. From 68a9dd9f9e72b2feaade18de2dbabc742406bdeb Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Thu, 1 May 2025 11:19:41 +0200 Subject: [PATCH 4/4] Address comments --- javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll b/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll index 4b53292e1482..dafc38ca8573 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll @@ -169,9 +169,7 @@ module Fastify { override string getRelativePath() { result = this.getArgument(0).getStringValue() } - override Http::RequestMethodName getHttpMethod() { - if this.getMethodName() = "all" then any() else result = this.getMethodName().toUpperCase() - } + override Http::RequestMethodName getHttpMethod() { result = this.getMethodName().toUpperCase() } } private class AddHookRouteSetup extends Routing::RouteSetup::MethodCall instanceof RouteSetup {