From 81cba7fa2f8e873ec8532004426aa17efafa2410 Mon Sep 17 00:00:00 2001 From: Napalys Date: Mon, 31 Mar 2025 13:50:56 +0200 Subject: [PATCH 01/12] Added test cases with missing alerts for `Request` and `NextRequest`. --- .../Request/app/api/proxy/route.serverSide.ts | 5 +++++ .../Request/app/api/proxy/route2.serverSide.ts | 8 ++++++++ .../Security/CWE-918/Request/package.json | 13 +++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Request/package.json diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts new file mode 100644 index 000000000000..bfee2442afb0 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts @@ -0,0 +1,5 @@ +export async function POST(req: Request) { + const { url } = await req.json(); // $ MISSING: Source[js/request-forgery] + const res = await fetch(url); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + return new Response(res.body, { headers: res.headers }); +} diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts new file mode 100644 index 000000000000..7b212a73542c --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts @@ -0,0 +1,8 @@ +import { NextRequest, NextResponse } from 'next/server'; + +export async function POST(req: NextRequest) { + const { url } = await req.json(); // $ MISSING: Source[js/request-forgery] + const res = await fetch(url); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const data = await res.text(); + return new NextResponse(data, { headers: res.headers }); +} diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/package.json b/javascript/ql/test/query-tests/Security/CWE-918/Request/package.json new file mode 100644 index 000000000000..329c7acb8239 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/package.json @@ -0,0 +1,13 @@ +{ + "name": "next-edge-proxy-app", + "version": "0.1.0", + "private": true, + "scripts": { + "dev": "next dev", + "build": "next build", + "start": "next start" + }, + "dependencies": { + "next": "15.1.7" + } +} From 63a3953b0cdab643d356ad4234c4f18510cfbc03 Mon Sep 17 00:00:00 2001 From: Napalys Date: Mon, 31 Mar 2025 13:57:43 +0200 Subject: [PATCH 02/12] Enhance Next.js API endpoint handling for compatibility with both Pages and App Router structures. --- .../lib/semmle/javascript/frameworks/Next.qll | 58 ++++++++++++++++++- .../Request/app/api/proxy/route.serverSide.ts | 4 +- .../app/api/proxy/route2.serverSide.ts | 4 +- .../Security/CWE-918/RequestForgery.expected | 20 +++++++ 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll index 8fce608a9704..6333c9442d85 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll @@ -213,10 +213,12 @@ module NextJS { /** * Gets a folder that contains API endpoints for a Next.js application. * These API endpoints act as Express-like route-handlers. + * It matches both the Pages Router (`pages/api/`) Next.js 12 or earlier and + * the App Router (`app/api/`) Next.js 13+ structures. */ Folder apiFolder() { - result = getANextPackage().getFile().getParentContainer().getFolder("pages").getFolder("api") - or + result = + getANextPackage().getFile().getParentContainer().getFolder(["pages", "app"]).getFolder("api") or result = apiFolder().getAFolder() } @@ -271,4 +273,56 @@ module NextJS { override string getCredentialsKind() { result = "jwt key" } } } + + /** + * A route handler for Next.js 13+ App Router API endpoints, which are defined by exporting + * HTTP method functions (like `GET`, `POST`, `PUT`, `DELETE`) from route.js files inside + * the `app/api/` directory. + */ + class NextAppRouteHandler extends DataFlow::FunctionNode, Http::Servers::StandardRouteHandler { + NextAppRouteHandler() { + exists(Module mod | mod.getFile().getParentContainer() = apiFolder() | + this = mod.getAnExportedValue(any(Http::RequestMethodName m)).getAFunctionValue() and + ( + this.getParameter(0).hasUnderlyingType("next/server", "NextRequest") + or + this.getParameter(0).hasUnderlyingType("Request") + ) + ) + } + + /** + * Gets the request parameter, which is either a `NextRequest` object (from `next/server`) or a standard web `Request` object. + */ + DataFlow::SourceNode getRequest() { result = this.getParameter(0) } + } + + /** + * A source of user-controlled data from a `NextRequest` object (from `next/server`) or a standard web `Request` object + * in a Next.js App Router route handler. + */ + class NextAppRequestSource extends Http::RequestInputAccess { + NextAppRouteHandler handler; + string kind; + + NextAppRequestSource() { + ( + this = + handler.getRequest().getAMethodCall(["json", "formData", "blob", "arrayBuffer", "text"]) + or + this = handler.getRequest().getAPropertyRead("body") + ) and + kind = "body" + or + this = handler.getRequest().getAPropertyRead(["url", "nextUrl"]) and kind = "url" + or + this = handler.getRequest().getAPropertyRead("headers") and kind = "headers" + } + + override string getKind() { result = kind } + + override Http::RouteHandler getRouteHandler() { result = handler } + + override string getSourceType() { result = "Next.js App Router request" } + } } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts index bfee2442afb0..f3d05b7e5aa2 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route.serverSide.ts @@ -1,5 +1,5 @@ export async function POST(req: Request) { - const { url } = await req.json(); // $ MISSING: Source[js/request-forgery] - const res = await fetch(url); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const { url } = await req.json(); // $ Source[js/request-forgery] + const res = await fetch(url); // $ Alert[js/request-forgery] Sink[js/request-forgery] return new Response(res.body, { headers: res.headers }); } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts index 7b212a73542c..051ba67e401f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/app/api/proxy/route2.serverSide.ts @@ -1,8 +1,8 @@ import { NextRequest, NextResponse } from 'next/server'; export async function POST(req: NextRequest) { - const { url } = await req.json(); // $ MISSING: Source[js/request-forgery] - const res = await fetch(url); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const { url } = await req.json(); // $ Source[js/request-forgery] + const res = await fetch(url); // $ Alert[js/request-forgery] Sink[js/request-forgery] const data = await res.text(); return new NextResponse(data, { headers: res.headers }); } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected index 78b02c5f7db4..b8436fa6722e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -1,4 +1,6 @@ #select +| Request/app/api/proxy/route2.serverSide.ts:5:21:5:30 | fetch(url) | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | URL | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | user-provided value | +| Request/app/api/proxy/route.serverSide.ts:3:21:3:30 | fetch(url) | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | URL | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | user-provided value | | 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 | | 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 | | 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 | @@ -27,6 +29,14 @@ | serverSide.js:125:5:128:6 | axios({ ... \\n }) | serverSide.js:123:29:123:35 | req.url | serverSide.js:127:14:127:20 | tainted | The $@ of this request depends on a $@. | serverSide.js:127:14:127:20 | tainted | URL | serverSide.js:123:29:123:35 | req.url | user-provided value | | serverSide.js:131:5:131:20 | axios.get(myUrl) | serverSide.js:123:29:123:35 | req.url | serverSide.js:131:15:131:19 | myUrl | The $@ of this request depends on a $@. | serverSide.js:131:15:131:19 | myUrl | URL | serverSide.js:123:29:123:35 | req.url | user-provided value | edges +| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | provenance | | +| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | provenance | | +| Request/app/api/proxy/route2.serverSide.ts:4:19:4:34 | await req.json() | Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | provenance | | +| Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | Request/app/api/proxy/route2.serverSide.ts:4:19:4:34 | await req.json() | provenance | | +| Request/app/api/proxy/route.serverSide.ts:2:9:2:15 | { url } | Request/app/api/proxy/route.serverSide.ts:2:9:2:34 | url | provenance | | +| Request/app/api/proxy/route.serverSide.ts:2:9:2:34 | url | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | provenance | | +| Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | Request/app/api/proxy/route.serverSide.ts:2:9:2:15 | { url } | provenance | | +| Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | provenance | | | apollo.serverSide.ts:7:36:7:44 | files | apollo.serverSide.ts:8:13:8:17 | files | provenance | | | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:7:36:7:44 | files | provenance | | | apollo.serverSide.ts:8:13:8:17 | files | apollo.serverSide.ts:8:28:8:31 | file | provenance | | @@ -91,6 +101,16 @@ edges | serverSide.js:130:9:130:45 | myUrl | serverSide.js:131:15:131:19 | myUrl | provenance | | | serverSide.js:130:37:130:43 | tainted | serverSide.js:130:9:130:45 | myUrl | provenance | | nodes +| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | semmle.label | { url } | +| Request/app/api/proxy/route2.serverSide.ts:4:9:4:34 | url | semmle.label | url | +| Request/app/api/proxy/route2.serverSide.ts:4:19:4:34 | await req.json() | semmle.label | await req.json() | +| Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | semmle.label | req.json() | +| Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | semmle.label | url | +| Request/app/api/proxy/route.serverSide.ts:2:9:2:15 | { url } | semmle.label | { url } | +| Request/app/api/proxy/route.serverSide.ts:2:9:2:34 | url | semmle.label | url | +| Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | semmle.label | await req.json() | +| Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | semmle.label | req.json() | +| Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | semmle.label | url | | apollo.serverSide.ts:7:36:7:44 | files | semmle.label | files | | apollo.serverSide.ts:7:36:7:44 | { files } | semmle.label | { files } | | apollo.serverSide.ts:8:13:8:17 | files | semmle.label | files | From 8acb0243ada91759d96d24121dc625d5b39db9a2 Mon Sep 17 00:00:00 2001 From: Napalys Date: Tue, 1 Apr 2025 14:14:27 +0200 Subject: [PATCH 03/12] Added test cases for `NextResponse` and `Response` --- .../ReflectedXss/ReflectedXss.expected | 17 ++++++++++ .../ReflectedXssWithCustomSanitizer.expected | 4 +++ .../CWE-079/ReflectedXss/app/api/route.ts | 30 +++++++++++++++++ .../ReflectedXss/app/api/routeNextRequest.ts | 32 +++++++++++++++++++ 4 files changed, 83 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/route.ts create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected index 75bef3e1b3b3..d4462e6064b5 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected @@ -27,6 +27,10 @@ | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to a $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value | | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to a $@. | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | user-provided value | | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | Cross-site scripting vulnerability due to a $@. | ReflectedXssGood3.js:135:15:135:27 | req.params.id | user-provided value | +| app/api/route.ts:5:18:5:21 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:5:18:5:21 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:13:18:13:21 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:13:18:13:21 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:25:18:25:21 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:25:18:25:21 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:29:25:29:28 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:29:25:29:28 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | etherpad.js:11:12:11:19 | response | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to a $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value | | formatting.js:6:14:6:47 | util.fo ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to a $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | formatting.js:7:14:7:53 | require ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to a $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | @@ -128,6 +132,12 @@ edges | ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:135:9:135:27 | url | provenance | | | ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:68:22:68:26 | value | provenance | | | ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | provenance | | +| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:5:18:5:21 | body | provenance | | +| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:13:18:13:21 | body | provenance | | +| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:25:18:25:21 | body | provenance | | +| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:29:25:29:28 | body | provenance | | +| app/api/route.ts:2:18:2:33 | await req.json() | app/api/route.ts:2:11:2:33 | body | provenance | | +| app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:2:18:2:33 | await req.json() | provenance | | | etherpad.js:9:5:9:53 | response | etherpad.js:11:12:11:19 | response | provenance | | | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:9:5:9:53 | response | provenance | | | formatting.js:4:9:4:29 | evil | formatting.js:6:43:6:46 | evil | provenance | | @@ -309,6 +319,13 @@ nodes | ReflectedXssGood3.js:135:15:135:27 | req.params.id | semmle.label | req.params.id | | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | semmle.label | escapeHtml3(url) | | ReflectedXssGood3.js:139:24:139:26 | url | semmle.label | url | +| app/api/route.ts:2:11:2:33 | body | semmle.label | body | +| app/api/route.ts:2:18:2:33 | await req.json() | semmle.label | await req.json() | +| app/api/route.ts:2:24:2:33 | req.json() | semmle.label | req.json() | +| app/api/route.ts:5:18:5:21 | body | semmle.label | body | +| app/api/route.ts:13:18:13:21 | body | semmle.label | body | +| app/api/route.ts:25:18:25:21 | body | semmle.label | body | +| app/api/route.ts:29:25:29:28 | body | semmle.label | body | | etherpad.js:9:5:9:53 | response | semmle.label | response | | etherpad.js:9:16:9:30 | req.query.jsonp | semmle.label | req.query.jsonp | | etherpad.js:11:12:11:19 | response | semmle.label | response | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected index 5532af3cf116..8fb9524ff45a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected @@ -26,6 +26,10 @@ | ReflectedXssContentTypes.js:39:13:39:35 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:39:23:39:35 | req.params.id | user-provided value | | ReflectedXssContentTypes.js:70:12:70:34 | "FOO: " ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXssContentTypes.js:70:22:70:34 | req.params.id | user-provided value | | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | Cross-site scripting vulnerability due to $@. | ReflectedXssGood3.js:135:15:135:27 | req.params.id | user-provided value | +| app/api/route.ts:5:18:5:21 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:13:18:13:21 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:25:18:25:21 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/route.ts:29:25:29:28 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | live-server.js:6:13:6:50 | ` ... /html>` | Cross-site scripting vulnerability due to $@. | live-server.js:4:21:4:27 | req.url | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/route.ts b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/route.ts new file mode 100644 index 000000000000..467f02a8ff8e --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/route.ts @@ -0,0 +1,30 @@ +export async function POST(req: Request) { + const body = await req.json(); // $ Source + + new Response(body, {headers: { 'Content-Type': 'application/json' }}); + new Response(body, {headers: { 'Content-Type': 'text/html' }}); // $ Alert + + const headers2 = new Headers(req.headers); + headers2.append('Content-Type', 'application/json'); + new Response(body, { headers: headers2 }); + + const headers3 = new Headers(req.headers); + headers3.append('Content-Type', 'text/html'); + new Response(body, { headers: headers3 }); // $ Alert + + const headers4 = new Headers({ + ...Object.fromEntries(req.headers), + 'Content-Type': 'application/json' + }); + new Response(body, { headers: headers4 }); + + const headers5 = new Headers({ + ...Object.fromEntries(req.headers), + 'Content-Type': 'text/html' + }); + new Response(body, { headers: headers5 }); // $ Alert + + const headers = new Headers(req.headers); + headers.set('Content-Type', 'text/html'); + return new Response(body, { headers }); // $ Alert +} diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts new file mode 100644 index 000000000000..758b91e1caa0 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts @@ -0,0 +1,32 @@ +import { NextRequest, NextResponse } from 'next/server'; + +export async function POST(req: NextRequest) { + const body = await req.json(); // $ MISSING: Source + + new NextResponse(body, {headers: { 'Content-Type': 'application/json' }}); + new NextResponse(body, {headers: { 'Content-Type': 'text/html' }}); // $ MISSING: Alert + + const headers2 = new Headers(req.headers); + headers2.append('Content-Type', 'application/json'); + new NextResponse(body, { headers: headers2 }); + + const headers3 = new Headers(req.headers); + headers3.append('Content-Type', 'text/html'); + new NextResponse(body, { headers: headers3 }); // $ MISSING: Alert + + const headers4 = new Headers({ + ...Object.fromEntries(req.headers), + 'Content-Type': 'application/json' + }); + new NextResponse(body, { headers: headers4 }); + + const headers5 = new Headers({ + ...Object.fromEntries(req.headers), + 'Content-Type': 'text/html' + }); + new NextResponse(body, { headers: headers5 }); // $ MISSING: Alert + + const headers = new Headers(req.headers); + headers.set('Content-Type', 'text/html'); + return new NextResponse(body, { headers }); // $ MISSING: Alert +} From 86b64afa13848bdc9bb6cdeeb1a7dee7fc2778d4 Mon Sep 17 00:00:00 2001 From: Napalys Date: Thu, 10 Apr 2025 15:06:44 +0200 Subject: [PATCH 04/12] Added `NextResponse` to the `ResponseCall` class it models similar near idential behaviour. --- .../javascript/frameworks/WebResponse.qll | 7 +++++-- .../CWE-079/ReflectedXss/ReflectedXss.expected | 17 +++++++++++++++++ .../ReflectedXssWithCustomSanitizer.expected | 4 ++++ .../ReflectedXss/app/api/routeNextRequest.ts | 10 +++++----- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll index 0e3f93d8099b..dfdee73c9d90 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll @@ -19,10 +19,13 @@ private class HeadersEntryPoint extends API::EntryPoint { } /** - * A call to the `Response` constructor. + * A call to the `Response` and `NextResponse` constructor. */ private class ResponseCall extends API::InvokeNode { - ResponseCall() { this = any(ResponseEntryPoint e).getANode().getAnInstantiation() } + ResponseCall() { + this = any(ResponseEntryPoint e).getANode().getAnInstantiation() or + this = API::moduleImport("next/server").getMember("NextResponse").getAnInstantiation() + } } /** diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected index d4462e6064b5..5681ae99aa85 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected @@ -31,6 +31,10 @@ | app/api/route.ts:13:18:13:21 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:13:18:13:21 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | app/api/route.ts:25:18:25:21 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:25:18:25:21 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | app/api/route.ts:29:25:29:28 | body | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:29:25:29:28 | body | Cross-site scripting vulnerability due to a $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:7:20:7:23 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:7:20:7:23 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:15:20:15:23 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:15:20:15:23 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:27:20:27:23 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:27:20:27:23 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:31:27:31:30 | body | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:31:27:31:30 | body | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | | etherpad.js:11:12:11:19 | response | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to a $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value | | formatting.js:6:14:6:47 | util.fo ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to a $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | formatting.js:7:14:7:53 | require ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to a $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | @@ -138,6 +142,12 @@ edges | app/api/route.ts:2:11:2:33 | body | app/api/route.ts:29:25:29:28 | body | provenance | | | app/api/route.ts:2:18:2:33 | await req.json() | app/api/route.ts:2:11:2:33 | body | provenance | | | app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:2:18:2:33 | await req.json() | provenance | | +| app/api/routeNextRequest.ts:4:9:4:31 | body | app/api/routeNextRequest.ts:7:20:7:23 | body | provenance | | +| app/api/routeNextRequest.ts:4:9:4:31 | body | app/api/routeNextRequest.ts:15:20:15:23 | body | provenance | | +| app/api/routeNextRequest.ts:4:9:4:31 | body | app/api/routeNextRequest.ts:27:20:27:23 | body | provenance | | +| app/api/routeNextRequest.ts:4:9:4:31 | body | app/api/routeNextRequest.ts:31:27:31:30 | body | provenance | | +| app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | app/api/routeNextRequest.ts:4:9:4:31 | body | provenance | | +| app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | provenance | | | etherpad.js:9:5:9:53 | response | etherpad.js:11:12:11:19 | response | provenance | | | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:9:5:9:53 | response | provenance | | | formatting.js:4:9:4:29 | evil | formatting.js:6:43:6:46 | evil | provenance | | @@ -326,6 +336,13 @@ nodes | app/api/route.ts:13:18:13:21 | body | semmle.label | body | | app/api/route.ts:25:18:25:21 | body | semmle.label | body | | app/api/route.ts:29:25:29:28 | body | semmle.label | body | +| app/api/routeNextRequest.ts:4:9:4:31 | body | semmle.label | body | +| app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | semmle.label | await req.json() | +| app/api/routeNextRequest.ts:4:22:4:31 | req.json() | semmle.label | req.json() | +| app/api/routeNextRequest.ts:7:20:7:23 | body | semmle.label | body | +| app/api/routeNextRequest.ts:15:20:15:23 | body | semmle.label | body | +| app/api/routeNextRequest.ts:27:20:27:23 | body | semmle.label | body | +| app/api/routeNextRequest.ts:31:27:31:30 | body | semmle.label | body | | etherpad.js:9:5:9:53 | response | semmle.label | response | | etherpad.js:9:16:9:30 | req.query.jsonp | semmle.label | req.query.jsonp | | etherpad.js:11:12:11:19 | response | semmle.label | response | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected index 8fb9524ff45a..b57d294c7a7f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected @@ -30,6 +30,10 @@ | app/api/route.ts:13:18:13:21 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | app/api/route.ts:25:18:25:21 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | | app/api/route.ts:29:25:29:28 | body | Cross-site scripting vulnerability due to $@. | app/api/route.ts:2:24:2:33 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:7:20:7:23 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:15:20:15:23 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:27:20:27:23 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | +| app/api/routeNextRequest.ts:31:27:31:30 | body | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value | | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value | | live-server.js:6:13:6:50 | ` ... /html>` | Cross-site scripting vulnerability due to $@. | live-server.js:4:21:4:27 | req.url | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts index 758b91e1caa0..91d9f4ecb51a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts @@ -1,10 +1,10 @@ import { NextRequest, NextResponse } from 'next/server'; export async function POST(req: NextRequest) { - const body = await req.json(); // $ MISSING: Source + const body = await req.json(); // $ Source new NextResponse(body, {headers: { 'Content-Type': 'application/json' }}); - new NextResponse(body, {headers: { 'Content-Type': 'text/html' }}); // $ MISSING: Alert + new NextResponse(body, {headers: { 'Content-Type': 'text/html' }}); // $ Alert const headers2 = new Headers(req.headers); headers2.append('Content-Type', 'application/json'); @@ -12,7 +12,7 @@ export async function POST(req: NextRequest) { const headers3 = new Headers(req.headers); headers3.append('Content-Type', 'text/html'); - new NextResponse(body, { headers: headers3 }); // $ MISSING: Alert + new NextResponse(body, { headers: headers3 }); // $ Alert const headers4 = new Headers({ ...Object.fromEntries(req.headers), @@ -24,9 +24,9 @@ export async function POST(req: NextRequest) { ...Object.fromEntries(req.headers), 'Content-Type': 'text/html' }); - new NextResponse(body, { headers: headers5 }); // $ MISSING: Alert + new NextResponse(body, { headers: headers5 }); // $ Alert const headers = new Headers(req.headers); headers.set('Content-Type', 'text/html'); - return new NextResponse(body, { headers }); // $ MISSING: Alert + return new NextResponse(body, { headers }); // $ Alert } From 208487f23618f3ca7249cba692537d72b18a7d14 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 08:39:47 +0200 Subject: [PATCH 05/12] Added `middleware` test --- .../Security/CWE-918/Request/middleware.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts new file mode 100644 index 000000000000..259e17d289dd --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts @@ -0,0 +1,12 @@ +import { NextRequest, NextResponse } from 'next/server'; + +export async function middleware(req: NextRequest) { + const target = req.nextUrl // $ MISSING : Source[js/request-forgery] + if (target) { + const res = await fetch(target) // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const data = await res.text() + return new NextResponse(data) + } + return NextResponse.next() +} + \ No newline at end of file From 734ad2d767597c57bf71847f3db0034c5619507a Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 08:43:08 +0200 Subject: [PATCH 06/12] Removed legacy `Consistency` check as it is redundant now with inline test expectations. --- .../Security/CWE-918/Consistency.expected | 2 -- .../Security/CWE-918/Consistency.ql | 25 ------------------- 2 files changed, 27 deletions(-) delete mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Consistency.expected delete mode 100644 javascript/ql/test/query-tests/Security/CWE-918/Consistency.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Consistency.expected b/javascript/ql/test/query-tests/Security/CWE-918/Consistency.expected deleted file mode 100644 index c2b63d9c9427..000000000000 --- a/javascript/ql/test/query-tests/Security/CWE-918/Consistency.expected +++ /dev/null @@ -1,2 +0,0 @@ -consistencyIssue -resultInWrongFile diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Consistency.ql b/javascript/ql/test/query-tests/Security/CWE-918/Consistency.ql deleted file mode 100644 index 5ae582db4915..000000000000 --- a/javascript/ql/test/query-tests/Security/CWE-918/Consistency.ql +++ /dev/null @@ -1,25 +0,0 @@ -import javascript -import semmle.javascript.security.dataflow.RequestForgeryQuery as RequestForgery -import semmle.javascript.security.dataflow.ClientSideRequestForgeryQuery as ClientSideRequestForgery -deprecated import utils.test.ConsistencyChecking - -query predicate resultInWrongFile(DataFlow::Node node) { - exists(string filePattern | - RequestForgery::RequestForgeryFlow::flowTo(node) and - filePattern = ".*serverSide.*" - or - ClientSideRequestForgery::ClientSideRequestForgeryFlow::flowTo(node) and - filePattern = ".*clientSide.*" - | - not node.getFile().getRelativePath().regexpMatch(filePattern) - ) -} - -deprecated class Consistency extends ConsistencyConfiguration { - Consistency() { this = "Consistency" } - - override DataFlow::Node getAnAlert() { - RequestForgery::RequestForgeryFlow::flowTo(result) or - ClientSideRequestForgery::ClientSideRequestForgeryFlow::flowTo(result) - } -} From 6e09a65da07fd6cbaa7b887327fa6d2935e5f086 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 08:43:36 +0200 Subject: [PATCH 07/12] Added support for `NextRequest` `middleware` SSRF. --- javascript/ql/lib/semmle/javascript/frameworks/Next.qll | 8 ++++++-- .../query-tests/Security/CWE-918/Request/middleware.ts | 4 ++-- .../query-tests/Security/CWE-918/RequestForgery.expected | 6 ++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll index 6333c9442d85..fad4f25e5f57 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll @@ -281,8 +281,12 @@ module NextJS { */ class NextAppRouteHandler extends DataFlow::FunctionNode, Http::Servers::StandardRouteHandler { NextAppRouteHandler() { - exists(Module mod | mod.getFile().getParentContainer() = apiFolder() | - this = mod.getAnExportedValue(any(Http::RequestMethodName m)).getAFunctionValue() and + exists(Module mod | + mod.getFile().getParentContainer() = apiFolder() or + mod.getFile().getBaseName() = ["middleware.ts", "middleware.js"] + | + this = + mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware"]).getAFunctionValue() and ( this.getParameter(0).hasUnderlyingType("next/server", "NextRequest") or diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts index 259e17d289dd..e6bb7f8db60f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts @@ -1,9 +1,9 @@ import { NextRequest, NextResponse } from 'next/server'; export async function middleware(req: NextRequest) { - const target = req.nextUrl // $ MISSING : Source[js/request-forgery] + const target = req.nextUrl // $ Source[js/request-forgery] if (target) { - const res = await fetch(target) // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const res = await fetch(target) // $ Alert[js/request-forgery] Sink[js/request-forgery] const data = await res.text() return new NextResponse(data) } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected index b8436fa6722e..cf3b225e3223 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -1,6 +1,7 @@ #select | Request/app/api/proxy/route2.serverSide.ts:5:21:5:30 | fetch(url) | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | URL | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | user-provided value | | Request/app/api/proxy/route.serverSide.ts:3:21:3:30 | fetch(url) | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | URL | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | user-provided value | +| Request/middleware.ts:6:25:6:37 | fetch(target) | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:6:31:6:36 | target | The $@ of this request depends on a $@. | Request/middleware.ts:6:31:6:36 | target | URL | Request/middleware.ts:4:20:4:30 | req.nextUrl | user-provided value | | 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 | | 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 | | 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 | @@ -37,6 +38,8 @@ edges | Request/app/api/proxy/route.serverSide.ts:2:9:2:34 | url | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | provenance | | | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | Request/app/api/proxy/route.serverSide.ts:2:9:2:15 | { url } | provenance | | | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | provenance | | +| Request/middleware.ts:4:11:4:30 | target | Request/middleware.ts:6:31:6:36 | target | provenance | | +| Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:4:11:4:30 | target | provenance | | | apollo.serverSide.ts:7:36:7:44 | files | apollo.serverSide.ts:8:13:8:17 | files | provenance | | | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:7:36:7:44 | files | provenance | | | apollo.serverSide.ts:8:13:8:17 | files | apollo.serverSide.ts:8:28:8:31 | file | provenance | | @@ -111,6 +114,9 @@ nodes | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | semmle.label | await req.json() | | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | semmle.label | req.json() | | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | semmle.label | url | +| Request/middleware.ts:4:11:4:30 | target | semmle.label | target | +| Request/middleware.ts:4:20:4:30 | req.nextUrl | semmle.label | req.nextUrl | +| Request/middleware.ts:6:31:6:36 | target | semmle.label | target | | apollo.serverSide.ts:7:36:7:44 | files | semmle.label | files | | apollo.serverSide.ts:7:36:7:44 | { files } | semmle.label | { files } | | apollo.serverSide.ts:8:13:8:17 | files | semmle.label | files | From 8674b61e5a886af7bd8f4d1c91cd5ed4670291ca Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 09:26:16 +0200 Subject: [PATCH 08/12] Added SSRF test case with `searchParams` for `NextRequest` --- .../test/query-tests/Security/CWE-918/Request/middleware.ts | 6 ++++++ .../query-tests/Security/CWE-918/RequestForgery.expected | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts index e6bb7f8db60f..2e6aa2746d6a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts @@ -2,11 +2,17 @@ import { NextRequest, NextResponse } from 'next/server'; export async function middleware(req: NextRequest) { const target = req.nextUrl // $ Source[js/request-forgery] + const target2 = target.searchParams.get('target'); // $ MISSING: Source[js/request-forgery] if (target) { const res = await fetch(target) // $ Alert[js/request-forgery] Sink[js/request-forgery] const data = await res.text() return new NextResponse(data) } + if (target2) { + const res = await fetch(target2); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const data = await res.text(); + return new NextResponse(data); + } return NextResponse.next() } \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected index cf3b225e3223..387a6590fbdc 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -1,7 +1,7 @@ #select | Request/app/api/proxy/route2.serverSide.ts:5:21:5:30 | fetch(url) | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | URL | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | user-provided value | | Request/app/api/proxy/route.serverSide.ts:3:21:3:30 | fetch(url) | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | URL | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | user-provided value | -| Request/middleware.ts:6:25:6:37 | fetch(target) | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:6:31:6:36 | target | The $@ of this request depends on a $@. | Request/middleware.ts:6:31:6:36 | target | URL | Request/middleware.ts:4:20:4:30 | req.nextUrl | user-provided value | +| Request/middleware.ts:7:25:7:37 | fetch(target) | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:7:31:7:36 | target | The $@ of this request depends on a $@. | Request/middleware.ts:7:31:7:36 | target | URL | Request/middleware.ts:4:20:4:30 | req.nextUrl | user-provided value | | 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 | | 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 | | 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 | @@ -38,7 +38,7 @@ edges | Request/app/api/proxy/route.serverSide.ts:2:9:2:34 | url | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | provenance | | | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | Request/app/api/proxy/route.serverSide.ts:2:9:2:15 | { url } | provenance | | | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | provenance | | -| Request/middleware.ts:4:11:4:30 | target | Request/middleware.ts:6:31:6:36 | target | provenance | | +| Request/middleware.ts:4:11:4:30 | target | Request/middleware.ts:7:31:7:36 | target | provenance | | | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:4:11:4:30 | target | provenance | | | apollo.serverSide.ts:7:36:7:44 | files | apollo.serverSide.ts:8:13:8:17 | files | provenance | | | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:7:36:7:44 | files | provenance | | @@ -116,7 +116,7 @@ nodes | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | semmle.label | url | | Request/middleware.ts:4:11:4:30 | target | semmle.label | target | | Request/middleware.ts:4:20:4:30 | req.nextUrl | semmle.label | req.nextUrl | -| Request/middleware.ts:6:31:6:36 | target | semmle.label | target | +| Request/middleware.ts:7:31:7:36 | target | semmle.label | target | | apollo.serverSide.ts:7:36:7:44 | files | semmle.label | files | | apollo.serverSide.ts:7:36:7:44 | { files } | semmle.label | { files } | | apollo.serverSide.ts:8:13:8:17 | files | semmle.label | files | From 678eccb417b32aa38d6b3a0426b448bc4c99612d Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 09:27:54 +0200 Subject: [PATCH 09/12] Added `searchParams.get` as potential source for SSRF --- .../ql/lib/semmle/javascript/frameworks/Next.qll | 12 +++++++++++- .../Security/CWE-918/Request/middleware.ts | 4 ++-- .../Security/CWE-918/RequestForgery.expected | 6 ++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll index fad4f25e5f57..2921b6c48503 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll @@ -318,7 +318,17 @@ module NextJS { ) and kind = "body" or - this = handler.getRequest().getAPropertyRead(["url", "nextUrl"]) and kind = "url" + ( + this = handler.getRequest().getAPropertyRead(["url", "nextUrl"]) + or + this = + handler + .getRequest() + .getAPropertyRead("nextUrl") + .getAPropertyRead("searchParams") + .getAMemberCall("get") + ) and + kind = "url" or this = handler.getRequest().getAPropertyRead("headers") and kind = "headers" } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts index 2e6aa2746d6a..3db3a4bae3b4 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts +++ b/javascript/ql/test/query-tests/Security/CWE-918/Request/middleware.ts @@ -2,14 +2,14 @@ import { NextRequest, NextResponse } from 'next/server'; export async function middleware(req: NextRequest) { const target = req.nextUrl // $ Source[js/request-forgery] - const target2 = target.searchParams.get('target'); // $ MISSING: Source[js/request-forgery] + const target2 = target.searchParams.get('target'); // $ Source[js/request-forgery] if (target) { const res = await fetch(target) // $ Alert[js/request-forgery] Sink[js/request-forgery] const data = await res.text() return new NextResponse(data) } if (target2) { - const res = await fetch(target2); // $ MISSING: Alert[js/request-forgery] Sink[js/request-forgery] + const res = await fetch(target2); // $ Alert[js/request-forgery] Sink[js/request-forgery] const data = await res.text(); return new NextResponse(data); } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected index 387a6590fbdc..b3d3055cd868 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -2,6 +2,7 @@ | Request/app/api/proxy/route2.serverSide.ts:5:21:5:30 | fetch(url) | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | URL | Request/app/api/proxy/route2.serverSide.ts:4:25:4:34 | req.json() | user-provided value | | Request/app/api/proxy/route.serverSide.ts:3:21:3:30 | fetch(url) | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | The $@ of this request depends on a $@. | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | URL | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | user-provided value | | Request/middleware.ts:7:25:7:37 | fetch(target) | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:7:31:7:36 | target | The $@ of this request depends on a $@. | Request/middleware.ts:7:31:7:36 | target | URL | Request/middleware.ts:4:20:4:30 | req.nextUrl | user-provided value | +| Request/middleware.ts:12:27:12:40 | fetch(target2) | Request/middleware.ts:5:21:5:53 | target. ... arget') | Request/middleware.ts:12:33:12:39 | target2 | The $@ of this request depends on a $@. | Request/middleware.ts:12:33:12:39 | target2 | URL | Request/middleware.ts:5:21:5:53 | target. ... arget') | user-provided value | | 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 | | 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 | | 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 | @@ -40,6 +41,8 @@ edges | Request/app/api/proxy/route.serverSide.ts:2:25:2:34 | req.json() | Request/app/api/proxy/route.serverSide.ts:2:19:2:34 | await req.json() | provenance | | | Request/middleware.ts:4:11:4:30 | target | Request/middleware.ts:7:31:7:36 | target | provenance | | | Request/middleware.ts:4:20:4:30 | req.nextUrl | Request/middleware.ts:4:11:4:30 | target | provenance | | +| Request/middleware.ts:5:11:5:53 | target2 | Request/middleware.ts:12:33:12:39 | target2 | provenance | | +| Request/middleware.ts:5:21:5:53 | target. ... arget') | Request/middleware.ts:5:11:5:53 | target2 | provenance | | | apollo.serverSide.ts:7:36:7:44 | files | apollo.serverSide.ts:8:13:8:17 | files | provenance | | | apollo.serverSide.ts:7:36:7:44 | { files } | apollo.serverSide.ts:7:36:7:44 | files | provenance | | | apollo.serverSide.ts:8:13:8:17 | files | apollo.serverSide.ts:8:28:8:31 | file | provenance | | @@ -116,7 +119,10 @@ nodes | Request/app/api/proxy/route.serverSide.ts:3:27:3:29 | url | semmle.label | url | | Request/middleware.ts:4:11:4:30 | target | semmle.label | target | | Request/middleware.ts:4:20:4:30 | req.nextUrl | semmle.label | req.nextUrl | +| Request/middleware.ts:5:11:5:53 | target2 | semmle.label | target2 | +| Request/middleware.ts:5:21:5:53 | target. ... arget') | semmle.label | target. ... arget') | | Request/middleware.ts:7:31:7:36 | target | semmle.label | target | +| Request/middleware.ts:12:33:12:39 | target2 | semmle.label | target2 | | apollo.serverSide.ts:7:36:7:44 | files | semmle.label | files | | apollo.serverSide.ts:7:36:7:44 | { files } | semmle.label | { files } | | apollo.serverSide.ts:8:13:8:17 | files | semmle.label | files | From 2c4b3527b4bee7f97b071614d97b6a544911c5a6 Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 09:38:23 +0200 Subject: [PATCH 10/12] Added change note --- javascript/ql/lib/change-notes/2025-04-11-nextrequest.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 javascript/ql/lib/change-notes/2025-04-11-nextrequest.md diff --git a/javascript/ql/lib/change-notes/2025-04-11-nextrequest.md b/javascript/ql/lib/change-notes/2025-04-11-nextrequest.md new file mode 100644 index 000000000000..9db5c34e51b2 --- /dev/null +++ b/javascript/ql/lib/change-notes/2025-04-11-nextrequest.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* Data passed to the [NextResponse](https://nextjs.org/docs/app/api-reference/functions/next-response) constructor is now treated as a sink for `js/reflected-xss`. +* Data received from [NextRequest](https://nextjs.org/docs/app/api-reference/functions/next-request) and [Request](https://developer.mozilla.org/en-US/docs/Web/API/Request) is now treated as a remote user input `source`. From 92e4f112c01256d7bde118ce3e30c4a7b4b7e418 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Fri, 11 Apr 2025 11:08:40 +0200 Subject: [PATCH 11/12] Update javascript/ql/lib/semmle/javascript/frameworks/Next.qll Co-authored-by: Asger F --- javascript/ql/lib/semmle/javascript/frameworks/Next.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll index 2921b6c48503..b31da84a3419 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll @@ -283,7 +283,7 @@ module NextJS { NextAppRouteHandler() { exists(Module mod | mod.getFile().getParentContainer() = apiFolder() or - mod.getFile().getBaseName() = ["middleware.ts", "middleware.js"] + mod.getFile().getStem() = "middleware" | this = mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware"]).getAFunctionValue() and From 11abbf8c4ae5267a17d3b4692c991a5a316c971d Mon Sep 17 00:00:00 2001 From: Napalys Date: Fri, 11 Apr 2025 11:19:12 +0200 Subject: [PATCH 12/12] Now `nextUrl` is of type `parameter` and loosen the restriction for `NextAppRouteHandler` --- .../lib/semmle/javascript/frameworks/Next.qll | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll index b31da84a3419..551d325f26af 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Next.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Next.qll @@ -286,12 +286,7 @@ module NextJS { mod.getFile().getStem() = "middleware" | this = - mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware"]).getAFunctionValue() and - ( - this.getParameter(0).hasUnderlyingType("next/server", "NextRequest") - or - this.getParameter(0).hasUnderlyingType("Request") - ) + mod.getAnExportedValue([any(Http::RequestMethodName m), "middleware"]).getAFunctionValue() ) } @@ -318,18 +313,17 @@ module NextJS { ) and kind = "body" or - ( - this = handler.getRequest().getAPropertyRead(["url", "nextUrl"]) - or - this = - handler - .getRequest() - .getAPropertyRead("nextUrl") - .getAPropertyRead("searchParams") - .getAMemberCall("get") - ) and + this = handler.getRequest().getAPropertyRead(["url", "nextUrl"]) and kind = "url" or + this = + handler + .getRequest() + .getAPropertyRead("nextUrl") + .getAPropertyRead("searchParams") + .getAMemberCall("get") and + kind = "parameter" + or this = handler.getRequest().getAPropertyRead("headers") and kind = "headers" }