Skip to content

Commit 3fbe348

Browse files
NapalysNapalys
authored
Merge pull request #19784 from Napalys/js/express_middleware
JS: Improve Express middleware taint tracking
2 parents b234d77 + c1b2fd8 commit 3fbe348

File tree

5 files changed

+67
-3
lines changed

5 files changed

+67
-3
lines changed

‎javascript/ql/lib/change-notes/2025-06-16-middleware-express.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Improved data flow tracking through middleware to handle default value and similar patterns.
5+
* Added `req._parsedUrl` as a remote input source.

‎javascript/ql/lib/semmle/javascript/Routing.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,7 @@ module Routing {
925925
private DataFlow::Node getAnAccessPathRhs(Node base, int n, string path) {
926926
// Assigned in the body of a route handler function, which is a middleware
927927
exists(RouteHandler handler | base = handler |
928-
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path) and
928+
result = AccessPath::getAnAssignmentTo(handler.getParameter(n).ref(), path).getALocalSource() and
929929
(
930930
exists(handler.getAContinuationInvocation())
931931
or

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,9 +618,9 @@ module Express {
618618
kind = "body" and
619619
this = ref.getAPropertyRead("body")
620620
or
621-
// `req.path`
621+
// `req.path` and `req._parsedUrl`
622622
kind = "url" and
623-
this = ref.getAPropertyRead("path")
623+
this = ref.getAPropertyRead(["path", "_parsedUrl"])
624624
)
625625
}
626626

‎javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
| apollo.serverSide.ts:8:39:8:64 | get(fil ... => {}) | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:8:43:8:50 | file.url | The $@ of this request depends on a $@. | apollo.serverSide.ts:8:43:8:50 | file.url | URL | apollo.serverSide.ts:7:36:7:44 | { files } | user-provided value |
77
| apollo.serverSide.ts:18:37:18:62 | get(fil ... => {}) | apollo.serverSide.ts:17:34:17:42 | { files } | apollo.serverSide.ts:18:41:18:48 | file.url | The $@ of this request depends on a $@. | apollo.serverSide.ts:18:41:18:48 | file.url | URL | apollo.serverSide.ts:17:34:17:42 | { files } | user-provided value |
88
| axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | axiosInterceptors.serverSide.js:19:21:19:28 | req.body | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | The $@ of this request depends on a $@. | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | endpoint | axiosInterceptors.serverSide.js:19:21:19:28 | req.body | user-provided value |
9+
| serverSide2.js:17:28:17:47 | axios.get(targetUrl) | serverSide2.js:10:25:10:31 | req.url | serverSide2.js:17:38:17:46 | targetUrl | The $@ of this request depends on a $@. | serverSide2.js:17:38:17:46 | targetUrl | URL | serverSide2.js:10:25:10:31 | req.url | user-provided value |
10+
| serverSide2.js:20:29:20:49 | axios.g ... etUrl1) | serverSide2.js:9:43:9:56 | req._parsedUrl | serverSide2.js:20:39:20:48 | targetUrl1 | The $@ of this request depends on a $@. | serverSide2.js:20:39:20:48 | targetUrl1 | URL | serverSide2.js:9:43:9:56 | req._parsedUrl | user-provided value |
11+
| serverSide2.js:23:29:23:49 | axios.g ... etUrl2) | serverSide2.js:22:24:22:30 | req.url | serverSide2.js:23:39:23:48 | targetUrl2 | The $@ of this request depends on a $@. | serverSide2.js:23:39:23:48 | targetUrl2 | URL | serverSide2.js:22:24:22:30 | req.url | user-provided value |
12+
| serverSide2.js:26:29:26:49 | axios.g ... etUrl3) | serverSide2.js:11:24:11:30 | req.url | serverSide2.js:26:39:26:48 | targetUrl3 | The $@ of this request depends on a $@. | serverSide2.js:26:39:26:48 | targetUrl3 | URL | serverSide2.js:11:24:11:30 | req.url | user-provided value |
913
| serverSide.js:18:5:18:20 | request(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:18:13:18:19 | tainted | The $@ of this request depends on a $@. | serverSide.js:18:13:18:19 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
1014
| serverSide.js:20:5:20:24 | request.get(tainted) | serverSide.js:14:29:14:35 | req.url | serverSide.js:20:17:20:23 | tainted | The $@ of this request depends on a $@. | serverSide.js:20:17:20:23 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
1115
| serverSide.js:24:5:24:20 | request(options) | serverSide.js:14:29:14:35 | req.url | serverSide.js:23:19:23:25 | tainted | The $@ of this request depends on a $@. | serverSide.js:23:19:23:25 | tainted | URL | serverSide.js:14:29:14:35 | req.url | user-provided value |
@@ -63,6 +67,18 @@ edges
6367
| axiosInterceptors.serverSide.js:19:21:19:28 | req.body | axiosInterceptors.serverSide.js:19:11:19:17 | { url } | provenance | |
6468
| axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | axiosInterceptors.serverSide.js:11:26:11:40 | userProvidedUrl | provenance | |
6569
| axiosInterceptors.serverSide.js:20:23:20:25 | url | axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | provenance | |
70+
| serverSide2.js:9:34:9:63 | qs.pars ... .query) | serverSide2.js:19:24:19:51 | req.par ... rsedUrl | provenance | |
71+
| serverSide2.js:9:43:9:56 | req._parsedUrl | serverSide2.js:9:34:9:63 | qs.pars ... .query) | provenance | |
72+
| serverSide2.js:10:25:10:31 | req.url | serverSide2.js:16:23:16:41 | req.parsedQuery.url | provenance | |
73+
| serverSide2.js:11:24:11:30 | req.url | serverSide2.js:25:24:25:41 | req.SomeObject.url | provenance | |
74+
| serverSide2.js:16:11:16:41 | targetUrl | serverSide2.js:17:38:17:46 | targetUrl | provenance | |
75+
| serverSide2.js:16:23:16:41 | req.parsedQuery.url | serverSide2.js:16:11:16:41 | targetUrl | provenance | |
76+
| serverSide2.js:19:11:19:55 | targetUrl1 | serverSide2.js:20:39:20:48 | targetUrl1 | provenance | |
77+
| serverSide2.js:19:24:19:51 | req.par ... rsedUrl | serverSide2.js:19:11:19:55 | targetUrl1 | provenance | |
78+
| serverSide2.js:22:11:22:36 | targetUrl2 | serverSide2.js:23:39:23:48 | targetUrl2 | provenance | |
79+
| serverSide2.js:22:24:22:30 | req.url | serverSide2.js:22:11:22:36 | targetUrl2 | provenance | |
80+
| serverSide2.js:25:11:25:47 | targetUrl3 | serverSide2.js:26:39:26:48 | targetUrl3 | provenance | |
81+
| serverSide2.js:25:24:25:41 | req.SomeObject.url | serverSide2.js:25:11:25:47 | targetUrl3 | provenance | |
6682
| serverSide.js:14:9:14:52 | tainted | serverSide.js:18:13:18:19 | tainted | provenance | |
6783
| serverSide.js:14:9:14:52 | tainted | serverSide.js:20:17:20:23 | tainted | provenance | |
6884
| serverSide.js:14:9:14:52 | tainted | serverSide.js:23:19:23:25 | tainted | provenance | |
@@ -163,6 +179,22 @@ nodes
163179
| axiosInterceptors.serverSide.js:19:21:19:28 | req.body | semmle.label | req.body |
164180
| axiosInterceptors.serverSide.js:20:5:20:25 | userProvidedUrl | semmle.label | userProvidedUrl |
165181
| axiosInterceptors.serverSide.js:20:23:20:25 | url | semmle.label | url |
182+
| serverSide2.js:9:34:9:63 | qs.pars ... .query) | semmle.label | qs.pars ... .query) |
183+
| serverSide2.js:9:43:9:56 | req._parsedUrl | semmle.label | req._parsedUrl |
184+
| serverSide2.js:10:25:10:31 | req.url | semmle.label | req.url |
185+
| serverSide2.js:11:24:11:30 | req.url | semmle.label | req.url |
186+
| serverSide2.js:16:11:16:41 | targetUrl | semmle.label | targetUrl |
187+
| serverSide2.js:16:23:16:41 | req.parsedQuery.url | semmle.label | req.parsedQuery.url |
188+
| serverSide2.js:17:38:17:46 | targetUrl | semmle.label | targetUrl |
189+
| serverSide2.js:19:11:19:55 | targetUrl1 | semmle.label | targetUrl1 |
190+
| serverSide2.js:19:24:19:51 | req.par ... rsedUrl | semmle.label | req.par ... rsedUrl |
191+
| serverSide2.js:20:39:20:48 | targetUrl1 | semmle.label | targetUrl1 |
192+
| serverSide2.js:22:11:22:36 | targetUrl2 | semmle.label | targetUrl2 |
193+
| serverSide2.js:22:24:22:30 | req.url | semmle.label | req.url |
194+
| serverSide2.js:23:39:23:48 | targetUrl2 | semmle.label | targetUrl2 |
195+
| serverSide2.js:25:11:25:47 | targetUrl3 | semmle.label | targetUrl3 |
196+
| serverSide2.js:25:24:25:41 | req.SomeObject.url | semmle.label | req.SomeObject.url |
197+
| serverSide2.js:26:39:26:48 | targetUrl3 | semmle.label | targetUrl3 |
166198
| serverSide.js:14:9:14:52 | tainted | semmle.label | tainted |
167199
| serverSide.js:14:19:14:42 | url.par ... , true) | semmle.label | url.par ... , true) |
168200
| serverSide.js:14:29:14:35 | req.url | semmle.label | req.url |

‎javascript/ql/test/query-tests/Security/CWE-918/serverSide2.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
const express = require('express');
2+
const axios = require('axios');
3+
const qs = require('qs');
4+
5+
const app = express();
6+
const PORT = 3000;
7+
8+
app.use((req, res, next) => {
9+
req.parsedQueryFromParsedUrl = qs.parse(req._parsedUrl.query); // $Source[js/request-forgery]
10+
req.parsedQuery.url = req.url || {}; // $Source[js/request-forgery]
11+
req.SomeObject.url = req.url; // $Source[js/request-forgery]
12+
next();
13+
});
14+
15+
app.get('/proxy', async (req, res) => {
16+
const targetUrl = req.parsedQuery.url;
17+
const response = await axios.get(targetUrl); // $Alert[js/request-forgery]
18+
19+
const targetUrl1 = req.parsedQueryFromParsedUrl.url;
20+
const response1 = await axios.get(targetUrl1); // $Alert[js/request-forgery]
21+
22+
const targetUrl2 = req.url || {}; // $Source[js/request-forgery]
23+
const response2 = await axios.get(targetUrl2); // $Alert[js/request-forgery]
24+
25+
const targetUrl3 = req.SomeObject.url || {};
26+
const response3 = await axios.get(targetUrl3); // $Alert[js/request-forgery]
27+
});

0 commit comments

Comments
 (0)