Skip to content

Commit a1dee15

Browse files
NapalysNapalys
committed
Modeled Response and NextResponse as potential sinks.
1 parent 08ba9a6 commit a1dee15

File tree

6 files changed

+118
-8
lines changed

6 files changed

+118
-8
lines changed

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

+41
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44

55
import javascript
6+
import semmle.javascript.security.dataflow.ReflectedXssCustomizations
67

78
/**
89
* Provides classes and predicates modeling [Next.js](https://www.npmjs.com/package/next).
@@ -324,4 +325,44 @@ module NextJS {
324325

325326
override string getSourceType() { result = "Next.js App Router request" }
326327
}
328+
329+
/**
330+
* Gets the headers value from the options object passed to a Next.js Response constructor.
331+
*/
332+
DataFlow::Node getContentTypeHeadersForResponse(DataFlow::InvokeNode responseNode) {
333+
exists(DataFlow::ObjectLiteralNode options |
334+
options.flowsTo(responseNode.getArgument(1)) and
335+
result = options.getAPropertyWrite("headers").getRhs()
336+
)
337+
}
338+
339+
/**
340+
* A sink representing response content in Next.js API routes that may be vulnerable
341+
* to XSS attacks.
342+
*
343+
* This class identifies responses created with the standard `Response` constructor
344+
* or Next.js `NextResponse`.
345+
*/
346+
class WebApiResponseSink extends Http::ResponseSendArgument {
347+
DataFlow::Node headers;
348+
349+
WebApiResponseSink() {
350+
exists(DataFlow::InvokeNode response, DataFlow::ObjectLiteralNode options |
351+
response =
352+
[
353+
DataFlow::globalVarRef("Response").getAnInstantiation(),
354+
API::moduleImport("next/server").getMember("NextResponse").getAnInstantiation()
355+
] and
356+
this = response.getArgument(0) and
357+
options.flowsTo(response.getArgument(1)) and
358+
headers = getContentTypeHeadersForResponse(response) and
359+
not exists(DataFlow::Node goodHeaders |
360+
goodHeaders = ReflectedXss::getGoodContentHeaders() and
361+
goodHeaders.getALocalSource().flowsTo(headers)
362+
)
363+
)
364+
}
365+
366+
override Http::RouteHandler getRouteHandler() { none() }
367+
}
327368
}

‎javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll

+35
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,41 @@ module ReflectedXss {
136136
}
137137
}
138138

139+
/**
140+
* Gets a data flow node representing headers that set content-type to a value
141+
* that is not susceptible to XSS attacks.
142+
*/
143+
DataFlow::Node getGoodContentHeaders() {
144+
// Case 1: Direct object literal with content-type `headers: { 'Content-Type': 'goodType' }`
145+
exists(DataFlow::ObjectLiteralNode headersObj, string name, string contentType |
146+
result = headersObj and
147+
name.toLowerCase() = "content-type" and
148+
headersObj.getAPropertyWrite(name).getRhs().mayHaveStringValue(contentType) and
149+
not contentType.toLowerCase().matches(xssUnsafeContentType() + "%")
150+
)
151+
or
152+
// Case 2: Headers with set/append methods `headers.append('Content-Type', 'goodType')`
153+
exists(DataFlow::MethodCallNode call, string contentType |
154+
call.getMethodName() = ["set", "append"] and
155+
call.getReceiver().getALocalSource() = result and
156+
call.getArgument(0).mayHaveStringValue(any(string s | s.toLowerCase() = "content-type")) and
157+
call.getArgument(1).mayHaveStringValue(contentType) and
158+
not contentType.toLowerCase().matches(xssUnsafeContentType() + "%")
159+
)
160+
or
161+
// Case 3: New Headers with initial content-type `new Headers({ 'Content-Type': 'goodType' })`
162+
exists(
163+
NewExpr headersNew, DataFlow::ObjectLiteralNode headersInit, string name, string contentType
164+
|
165+
result.asExpr() = headersNew and
166+
headersNew.getCalleeName() = "Headers" and
167+
headersInit.flowsTo(DataFlow::valueNode(headersNew.getArgument(0))) and
168+
name.toLowerCase() = "content-type" and
169+
headersInit.getAPropertyWrite(name).getRhs().mayHaveStringValue(contentType) and
170+
not contentType.toLowerCase().matches(xssUnsafeContentType() + "%")
171+
)
172+
}
173+
139174
private class SinkFromModel extends Sink {
140175
SinkFromModel() { this = ModelOutput::getASinkNode("html-injection").asSink() }
141176
}

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected

+28
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@
2727
| 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 |
2828
| 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 |
2929
| 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 |
30+
| 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 |
31+
| 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 |
32+
| 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 |
33+
| 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 |
34+
| app/api/routeNextRequest.ts:5:20:5:23 | data | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:5:20:5:23 | data | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
35+
| app/api/routeNextRequest.ts:6:27:6:30 | data | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:6:27:6:30 | data | Cross-site scripting vulnerability due to a $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
3036
| 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 |
3137
| 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 |
3238
| 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 |
@@ -119,6 +125,16 @@ edges
119125
| ReflectedXssGood3.js:135:15:135:27 | req.params.id | ReflectedXssGood3.js:135:9:135:27 | url | provenance | |
120126
| ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:68:22:68:26 | value | provenance | |
121127
| ReflectedXssGood3.js:139:24:139:26 | url | ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | provenance | |
128+
| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:5:18:5:21 | body | provenance | |
129+
| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:13:18:13:21 | body | provenance | |
130+
| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:25:18:25:21 | body | provenance | |
131+
| app/api/route.ts:2:11:2:33 | body | app/api/route.ts:29:25:29:28 | body | provenance | |
132+
| app/api/route.ts:2:18:2:33 | await req.json() | app/api/route.ts:2:11:2:33 | body | provenance | |
133+
| app/api/route.ts:2:24:2:33 | req.json() | app/api/route.ts:2:18:2:33 | await req.json() | provenance | |
134+
| app/api/routeNextRequest.ts:4:9:4:31 | data | app/api/routeNextRequest.ts:5:20:5:23 | data | provenance | |
135+
| app/api/routeNextRequest.ts:4:9:4:31 | data | app/api/routeNextRequest.ts:6:27:6:30 | data | provenance | |
136+
| app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | app/api/routeNextRequest.ts:4:9:4:31 | data | provenance | |
137+
| app/api/routeNextRequest.ts:4:22:4:31 | req.json() | app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | provenance | |
122138
| etherpad.js:9:5:9:53 | response | etherpad.js:11:12:11:19 | response | provenance | |
123139
| etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:9:5:9:53 | response | provenance | |
124140
| formatting.js:4:9:4:29 | evil | formatting.js:6:43:6:46 | evil | provenance | |
@@ -290,6 +306,18 @@ nodes
290306
| ReflectedXssGood3.js:135:15:135:27 | req.params.id | semmle.label | req.params.id |
291307
| ReflectedXssGood3.js:139:12:139:27 | escapeHtml3(url) | semmle.label | escapeHtml3(url) |
292308
| ReflectedXssGood3.js:139:24:139:26 | url | semmle.label | url |
309+
| app/api/route.ts:2:11:2:33 | body | semmle.label | body |
310+
| app/api/route.ts:2:18:2:33 | await req.json() | semmle.label | await req.json() |
311+
| app/api/route.ts:2:24:2:33 | req.json() | semmle.label | req.json() |
312+
| app/api/route.ts:5:18:5:21 | body | semmle.label | body |
313+
| app/api/route.ts:13:18:13:21 | body | semmle.label | body |
314+
| app/api/route.ts:25:18:25:21 | body | semmle.label | body |
315+
| app/api/route.ts:29:25:29:28 | body | semmle.label | body |
316+
| app/api/routeNextRequest.ts:4:9:4:31 | data | semmle.label | data |
317+
| app/api/routeNextRequest.ts:4:16:4:31 | await req.json() | semmle.label | await req.json() |
318+
| app/api/routeNextRequest.ts:4:22:4:31 | req.json() | semmle.label | req.json() |
319+
| app/api/routeNextRequest.ts:5:20:5:23 | data | semmle.label | data |
320+
| app/api/routeNextRequest.ts:6:27:6:30 | data | semmle.label | data |
293321
| etherpad.js:9:5:9:53 | response | semmle.label | response |
294322
| etherpad.js:9:16:9:30 | req.query.jsonp | semmle.label | req.query.jsonp |
295323
| etherpad.js:11:12:11:19 | response | semmle.label | response |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected

+6
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@
2626
| 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 |
2727
| 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 |
2828
| 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 |
29+
| 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 |
30+
| 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 |
31+
| 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 |
32+
| 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 |
33+
| app/api/routeNextRequest.ts:5:20:5:23 | data | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
34+
| app/api/routeNextRequest.ts:6:27:6:30 | data | Cross-site scripting vulnerability due to $@. | app/api/routeNextRequest.ts:4:22:4:31 | req.json() | user-provided value |
2935
| 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 |
3036
| 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 |
3137
| live-server.js:6:13:6:50 | `<html> ... /html>` | Cross-site scripting vulnerability due to $@. | live-server.js:4:21:4:27 | req.url | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/route.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
export async function POST(req: Request) {
2-
const body = await req.json(); // $ MISSING: Source
2+
const body = await req.json(); // $ Source
33

44
new Response(body, {headers: { 'Content-Type': 'application/json' }}); // This is okay, since content type is set to application/json
5-
new Response(body, {headers: { 'Content-Type': 'text/html' }}); // $ MISSING: Alert
5+
new Response(body, {headers: { 'Content-Type': 'text/html' }}); // $ Alert
66

77
const headers2 = new Headers(req.headers);
88
headers2.append('Content-Type', 'application/json');
99
new Response(body, { headers: headers2 }); // This is okay, since content type is set to application/json
1010

1111
const headers3 = new Headers(req.headers);
1212
headers3.append('Content-Type', 'text/html');
13-
new Response(body, { headers: headers3 }); // $ MISSING: Alert
13+
new Response(body, { headers: headers3 }); // $ Alert
1414

1515
const headers4 = new Headers({
1616
...Object.fromEntries(req.headers),
@@ -22,9 +22,9 @@ export async function POST(req: Request) {
2222
...Object.fromEntries(req.headers),
2323
'Content-Type': 'text/html'
2424
});
25-
new Response(body, { headers: headers5 }); // $ MISSING: Alert
25+
new Response(body, { headers: headers5 }); // $ Alert
2626

2727
const headers = new Headers(req.headers);
2828
headers.set('Content-Type', 'text/html');
29-
return new Response(body, { headers }); // $ MISSING: Alert
29+
return new Response(body, { headers }); // $ Alert
3030
}

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/app/api/routeNextRequest.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { NextRequest, NextResponse } from 'next/server';
22

33
export async function POST(req: NextRequest) {
4-
const data = await req.json(); // $ MISSING: Source
5-
new NextResponse(data, {headers: { 'Content-Type': 'text/html' }}); // $ MISSING: Alert
6-
return new NextResponse(data, { headers: req.headers }); // $ MISSING: Alert
4+
const data = await req.json(); // $ Source
5+
new NextResponse(data, {headers: { 'Content-Type': 'text/html' }}); // $ Alert
6+
return new NextResponse(data, { headers: req.headers }); // $ Alert
77
}

0 commit comments

Comments
 (0)