Skip to content

Commit 5c3556d

Browse files
committed
Add user-controlled property tracking and update code injection alerts in Fastify hooks
1 parent 9b194ea commit 5c3556d

File tree

4 files changed

+40
-3
lines changed

4 files changed

+40
-3
lines changed

Diff for: javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll

+28
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,20 @@ module Fastify {
238238
}
239239
}
240240

241+
/**
242+
* Gets the property name where user-controlled input is written to a request or response object
243+
* in a route handler. This is used to track taint flow through request and response object properties.
244+
*/
245+
private string getUserControlledPropertyName() {
246+
exists(DataFlow::PropWrite write, DataFlow::Node source, RouteHandler rh |
247+
write.getBase*() =
248+
[rh.getARequestSource().ref().getALocalUse(), rh.getAResponseSource().ref().getALocalUse()] and
249+
write.getPropertyName() = result and
250+
write.getRhs() = source and
251+
source = any(Http::RequestInputAccess ria).getASuccessor*()
252+
)
253+
}
254+
241255
/**
242256
* An access to a user-controlled Fastify request input.
243257
*/
@@ -252,6 +266,20 @@ module Fastify {
252266
or
253267
kind = "body" and
254268
name = "body"
269+
or
270+
kind = "stored" and
271+
name = getUserControlledPropertyName()
272+
)
273+
or
274+
// Handle reading from reply object with user input stored on it
275+
exists(string name |
276+
(
277+
this = rh.getAResponseSource().ref().getAPropertyRead(name)
278+
or
279+
this = rh.getAResponseSource().ref().getAPropertyRead+().getAPropertyRead(name)
280+
) and
281+
kind = "stored" and
282+
name = getUserControlledPropertyName()
255283
)
256284
}
257285

Diff for: javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/CodeInjection.expected

+6
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@
5151
| fastify.js:58:44:58:52 | userInput | fastify.js:57:21:57:39 | request.query.input | fastify.js:58:44:58:52 | userInput | This code execution depends on a $@. | fastify.js:57:21:57:39 | request.query.input | user-provided value |
5252
| fastify.js:59:23:59:31 | userInput | fastify.js:57:21:57:33 | request.query | fastify.js:59:23:59:31 | userInput | This code execution depends on a $@. | fastify.js:57:21:57:33 | request.query | user-provided value |
5353
| fastify.js:59:23:59:31 | userInput | fastify.js:57:21:57:39 | request.query.input | fastify.js:59:23:59:31 | userInput | This code execution depends on a $@. | fastify.js:57:21:57:39 | request.query.input | user-provided value |
54+
| fastify.js:71:34:71:51 | request.storedCode | fastify.js:71:34:71:51 | request.storedCode | fastify.js:71:34:71:51 | request.storedCode | This code execution depends on a $@. | fastify.js:71:34:71:51 | request.storedCode | user-provided value |
55+
| fastify.js:84:30:84:43 | reply.userCode | fastify.js:84:30:84:43 | reply.userCode | fastify.js:84:30:84:43 | reply.userCode | This code execution depends on a $@. | fastify.js:84:30:84:43 | reply.userCode | user-provided value |
56+
| fastify.js:99:30:99:52 | reply.l ... tedCode | fastify.js:99:30:99:52 | reply.l ... tedCode | fastify.js:99:30:99:52 | reply.l ... tedCode | This code execution depends on a $@. | fastify.js:99:30:99:52 | reply.l ... tedCode | user-provided value |
5457
| 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 |
5558
| 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 |
5659
| 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 |
@@ -247,6 +250,9 @@ nodes
247250
| fastify.js:57:21:57:39 | request.query.input | semmle.label | request.query.input |
248251
| fastify.js:58:44:58:52 | userInput | semmle.label | userInput |
249252
| fastify.js:59:23:59:31 | userInput | semmle.label | userInput |
253+
| fastify.js:71:34:71:51 | request.storedCode | semmle.label | request.storedCode |
254+
| fastify.js:84:30:84:43 | reply.userCode | semmle.label | reply.userCode |
255+
| fastify.js:99:30:99:52 | reply.l ... tedCode | semmle.label | reply.l ... tedCode |
250256
| module.js:9:16:9:29 | req.query.code | semmle.label | req.query.code |
251257
| module.js:11:17:11:30 | req.query.code | semmle.label | req.query.code |
252258
| react-native.js:7:7:7:33 | tainted | semmle.label | tainted |

Diff for: javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/HeuristicSourceCodeInjection.expected

+3
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ nodes
161161
| fastify.js:57:21:57:39 | request.query.input | semmle.label | request.query.input |
162162
| fastify.js:58:44:58:52 | userInput | semmle.label | userInput |
163163
| fastify.js:59:23:59:31 | userInput | semmle.label | userInput |
164+
| fastify.js:71:34:71:51 | request.storedCode | semmle.label | request.storedCode |
165+
| fastify.js:84:30:84:43 | reply.userCode | semmle.label | reply.userCode |
166+
| fastify.js:99:30:99:52 | reply.l ... tedCode | semmle.label | reply.l ... tedCode |
164167
| module.js:9:16:9:29 | req.query.code | semmle.label | req.query.code |
165168
| module.js:11:17:11:30 | req.query.code | semmle.label | req.query.code |
166169
| react-native.js:7:7:7:33 | tainted | semmle.label | tainted |

Diff for: javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/fastify.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ fastify.addHook('preHandler', async (request, reply) => {
6868
fastify.get('/flow-through-request', async (request, reply) => {
6969
// Use the stored code from previous hook
7070
if (request.storedCode) {
71-
const evaluatedResult = eval(request.storedCode); // $ MISSING: Alert[js/code-injection]
71+
const evaluatedResult = eval(request.storedCode); // $ Alert[js/code-injection]
7272
return { result: evaluatedResult };
7373
}
7474
return { result: null };
@@ -81,7 +81,7 @@ fastify.addHook('onRequest', async (request, reply) => {
8181
fastify.get('/flow-through-reply', async (request, reply) => {
8282
// Use the code stored in reply object
8383
if (reply.userCode) {
84-
const replyResult = eval(reply.userCode); // $ MISSING: Alert[js/code-injection]
84+
const replyResult = eval(reply.userCode); // $ Alert[js/code-injection]
8585
return { result: replyResult };
8686
}
8787
return { result: null };
@@ -96,7 +96,7 @@ fastify.addHook('onRequest', async (request, reply) => {
9696
fastify.get('/flow-through-reply', async (request, reply) => {
9797
// Use the code stored in reply object
9898
if (reply.locals && reply.locals.nestedCode) {
99-
const replyResult = eval(reply.locals.nestedCode); // $ MISSING: Alert[js/code-injection]
99+
const replyResult = eval(reply.locals.nestedCode); // $ Alert[js/code-injection]
100100
return { result: replyResult };
101101
}
102102
return { result: null };

0 commit comments

Comments
 (0)