Skip to content

Commit eac14b9

Browse files
asgerfasgerf
authored
Merge pull request #19200 from asgerf/js/web-response
JS: Add sinks for calls to 'new Response()'
2 parents 7f7fca9 + da7d6d3 commit eac14b9

File tree

8 files changed

+206
-9
lines changed

8 files changed

+206
-9
lines changed

‎javascript/ql/lib/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ import semmle.javascript.frameworks.UriLibraries
136136
import semmle.javascript.frameworks.Vue
137137
import semmle.javascript.frameworks.Vuex
138138
import semmle.javascript.frameworks.Webix
139+
import semmle.javascript.frameworks.WebResponse
139140
import semmle.javascript.frameworks.WebSocket
140141
import semmle.javascript.frameworks.XmlParsers
141142
import semmle.javascript.frameworks.xUnit

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ module Http {
108108
* Gets the route handler that sends this expression.
109109
*/
110110
abstract RouteHandler getRouteHandler();
111+
112+
/**
113+
* Gets a header definition associated with this response body, if it they are provided
114+
* by the same call.
115+
*/
116+
HeaderDefinition getAnAssociatedHeaderDefinition() { none() }
111117
}
112118

113119
/**

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/**
2+
* Models the `Request` and `Response` objects from the Web standards.
3+
*/
4+
5+
private import javascript
6+
7+
/** Treats `Response` as an entry point for API graphs. */
8+
private class ResponseEntryPoint extends API::EntryPoint {
9+
ResponseEntryPoint() { this = "global.Response" }
10+
11+
override DataFlow::SourceNode getASource() { result = DataFlow::globalVarRef("Response") }
12+
}
13+
14+
/** Treats `Headers` as an entry point for API graphs. */
15+
private class HeadersEntryPoint extends API::EntryPoint {
16+
HeadersEntryPoint() { this = "global.Headers" }
17+
18+
override DataFlow::SourceNode getASource() { result = DataFlow::globalVarRef("Headers") }
19+
}
20+
21+
/**
22+
* A call to the `Response` constructor.
23+
*/
24+
private class ResponseCall extends API::InvokeNode {
25+
ResponseCall() { this = any(ResponseEntryPoint e).getANode().getAnInstantiation() }
26+
}
27+
28+
/**
29+
* A call to the `Headers` constructor.
30+
*/
31+
private class HeadersCall extends API::InvokeNode {
32+
HeadersCall() { this = any(HeadersEntryPoint e).getANode().getAnInstantiation() }
33+
}
34+
35+
/**
36+
* The `headers` in `new Response(body, { headers })`
37+
*/
38+
private class ResponseArgumentHeaders extends Http::HeaderDefinition {
39+
private ResponseCall response;
40+
private API::Node headerNode;
41+
42+
ResponseArgumentHeaders() {
43+
headerNode = response.getParameter(1).getMember("headers") and
44+
this = headerNode.asSink()
45+
}
46+
47+
ResponseCall getResponse() { result = response }
48+
49+
/**
50+
* Gets a call to `new Headers()` that is passed as the headers to this call.
51+
*/
52+
private HeadersCall getHeadersCall() { headerNode.refersTo(result.getReturn()) }
53+
54+
/**
55+
* Gets an object whose properties are interpreted as headers, such as `{'content-type': 'foo'}`.
56+
*/
57+
private API::Node getAPlainHeaderObject() {
58+
// new Response(body, {...})
59+
result = headerNode
60+
or
61+
// new Response(body, new Headers({...}))
62+
result = this.getHeadersCall().getParameter(0)
63+
}
64+
65+
private API::Node getHeaderNode(string headerName) {
66+
exists(string prop |
67+
result = this.getAPlainHeaderObject().getMember(prop) and
68+
headerName = prop.toLowerCase()
69+
)
70+
or
71+
exists(API::CallNode append |
72+
append = this.getHeadersCall().getReturn().getMember(["append", "set"]).getACall() and
73+
headerName = append.getArgument(0).getStringValue().toLowerCase() and
74+
result = append.getParameter(1)
75+
)
76+
}
77+
78+
override predicate defines(string headerName, string headerValue) {
79+
this.getHeaderNode(headerName).getAValueReachingSink().getStringValue() = headerValue
80+
}
81+
82+
override string getAHeaderName() { exists(this.getHeaderNode(result)) }
83+
84+
override Http::RouteHandler getRouteHandler() { none() }
85+
}
86+
87+
/**
88+
* Data passed as the body in `new Response(body, ...)`.
89+
*/
90+
private class ResponseSink extends Http::ResponseSendArgument {
91+
private ResponseCall response;
92+
93+
ResponseSink() { this = response.getArgument(0) }
94+
95+
override Http::RouteHandler getRouteHandler() { none() }
96+
97+
override ResponseArgumentHeaders getAnAssociatedHeaderDefinition() {
98+
result.getResponse() = response
99+
}
100+
}

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ module ReflectedXss {
3232
* Gets a HeaderDefinition that defines a XSS safe content-type for `send`.
3333
*/
3434
Http::HeaderDefinition getAXssSafeHeaderDefinition(Http::ResponseSendArgument send) {
35-
exists(Http::RouteHandler h |
36-
send.getRouteHandler() = h and
37-
result = xssSafeContentTypeHeader(h)
38-
|
39-
// The HeaderDefinition affects a response sent at `send`.
35+
isSafeContentTypeHeader(result) and
36+
(
37+
result = send.getAnAssociatedHeaderDefinition()
38+
or
39+
result = send.getRouteHandler().getAResponseHeader("content-type") and
4040
headerAffects(result, send)
4141
)
4242
}
@@ -54,14 +54,20 @@ module ReflectedXss {
5454
]
5555
}
5656

57+
private predicate isSafeContentTypeHeader(Http::HeaderDefinition header) {
58+
header.getAHeaderName() = "content-type" and
59+
not exists(string tp | header.defines("content-type", tp) |
60+
tp.toLowerCase().matches(xssUnsafeContentType() + "%")
61+
)
62+
}
63+
5764
/**
65+
* DEPRECATED. Use `getAXssSafeHeaderDefinition` instead.
5866
* Holds if `h` may send a response with a content type that is safe for XSS.
5967
*/
60-
Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) {
68+
deprecated Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) {
6169
result = h.getAResponseHeader("content-type") and
62-
not exists(string tp | result.defines("content-type", tp) |
63-
tp.toLowerCase().matches(xssUnsafeContentType() + "%")
64-
)
70+
isSafeContentTypeHeader(result)
6571
}
6672

6773
/**
@@ -80,6 +86,8 @@ module ReflectedXss {
8086
dominatingHeader.getBasicBlock().(ReachableBasicBlock).dominates(sender.getBasicBlock())
8187
)
8288
)
89+
or
90+
header = sender.getAnAssociatedHeaderDefinition()
8391
}
8492

8593
bindingset[headerBlock]

‎javascript/ql/src/change-notes/2025-04-09-web-response.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Data passed to the [Response](https://developer.mozilla.org/en-US/docs/Web/API/Response) constructor is now treated as a sink for `js/reflected-xss`.

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@
4040
| partial.js:28:14:28:18 | x + y | partial.js:31:47:31:53 | req.url | partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to a $@. | partial.js:31:47:31:53 | req.url | user-provided value |
4141
| partial.js:37:14:37:18 | x + y | partial.js:40:43:40:49 | req.url | partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to a $@. | partial.js:40:43:40:49 | req.url | user-provided value |
4242
| promises.js:6:25:6:25 | x | promises.js:5:44:5:57 | req.query.data | promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to a $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
43+
| response-object.js:9:18:9:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:9:18:9:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
44+
| response-object.js:10:18:10:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:10:18:10:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
45+
| response-object.js:11:18:11:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:11:18:11:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
46+
| response-object.js:14:18:14:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:14:18:14:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
47+
| response-object.js:17:18:17:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:17:18:17:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
48+
| response-object.js:23:18:23:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:23:18:23:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
49+
| response-object.js:26:18:26:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:26:18:26:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
50+
| response-object.js:34:18:34:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:34:18:34:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
51+
| response-object.js:38:18:38:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:38:18:38:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4352
| tst2.js:7:12:7:12 | p | tst2.js:6:9:6:9 | p | tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to a $@. | tst2.js:6:9:6:9 | p | user-provided value |
4453
| tst2.js:8:12:8:12 | r | tst2.js:6:12:6:15 | q: r | tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to a $@. | tst2.js:6:12:6:15 | q: r | user-provided value |
4554
| tst2.js:18:12:18:12 | p | tst2.js:14:9:14:9 | p | tst2.js:18:12:18:12 | p | Cross-site scripting vulnerability due to a $@. | tst2.js:14:9:14:9 | p | user-provided value |
@@ -149,6 +158,16 @@ edges
149158
| promises.js:5:36:5:42 | [post update] resolve [resolve-value] | promises.js:5:16:5:22 | resolve [Return] [resolve-value] | provenance | |
150159
| promises.js:5:44:5:57 | req.query.data | promises.js:5:36:5:42 | [post update] resolve [resolve-value] | provenance | |
151160
| promises.js:6:11:6:11 | x | promises.js:6:25:6:25 | x | provenance | |
161+
| response-object.js:7:11:7:25 | data | response-object.js:9:18:9:21 | data | provenance | |
162+
| response-object.js:7:11:7:25 | data | response-object.js:10:18:10:21 | data | provenance | |
163+
| response-object.js:7:11:7:25 | data | response-object.js:11:18:11:21 | data | provenance | |
164+
| response-object.js:7:11:7:25 | data | response-object.js:14:18:14:21 | data | provenance | |
165+
| response-object.js:7:11:7:25 | data | response-object.js:17:18:17:21 | data | provenance | |
166+
| response-object.js:7:11:7:25 | data | response-object.js:23:18:23:21 | data | provenance | |
167+
| response-object.js:7:11:7:25 | data | response-object.js:26:18:26:21 | data | provenance | |
168+
| response-object.js:7:11:7:25 | data | response-object.js:34:18:34:21 | data | provenance | |
169+
| response-object.js:7:11:7:25 | data | response-object.js:38:18:38:21 | data | provenance | |
170+
| response-object.js:7:18:7:25 | req.body | response-object.js:7:11:7:25 | data | provenance | |
152171
| tst2.js:6:7:6:30 | p | tst2.js:7:12:7:12 | p | provenance | |
153172
| tst2.js:6:7:6:30 | r | tst2.js:8:12:8:12 | r | provenance | |
154173
| tst2.js:6:9:6:9 | p | tst2.js:6:7:6:30 | p | provenance | |
@@ -332,6 +351,17 @@ nodes
332351
| promises.js:5:44:5:57 | req.query.data | semmle.label | req.query.data |
333352
| promises.js:6:11:6:11 | x | semmle.label | x |
334353
| promises.js:6:25:6:25 | x | semmle.label | x |
354+
| response-object.js:7:11:7:25 | data | semmle.label | data |
355+
| response-object.js:7:18:7:25 | req.body | semmle.label | req.body |
356+
| response-object.js:9:18:9:21 | data | semmle.label | data |
357+
| response-object.js:10:18:10:21 | data | semmle.label | data |
358+
| response-object.js:11:18:11:21 | data | semmle.label | data |
359+
| response-object.js:14:18:14:21 | data | semmle.label | data |
360+
| response-object.js:17:18:17:21 | data | semmle.label | data |
361+
| response-object.js:23:18:23:21 | data | semmle.label | data |
362+
| response-object.js:26:18:26:21 | data | semmle.label | data |
363+
| response-object.js:34:18:34:21 | data | semmle.label | data |
364+
| response-object.js:38:18:38:21 | data | semmle.label | data |
335365
| tst2.js:6:7:6:30 | p | semmle.label | p |
336366
| tst2.js:6:7:6:30 | r | semmle.label | r |
337367
| tst2.js:6:9:6:9 | p | semmle.label | p |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,15 @@
3838
| partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:31:47:31:53 | req.url | user-provided value |
3939
| partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:40:43:40:49 | req.url | user-provided value |
4040
| promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
41+
| response-object.js:9:18:9:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
42+
| response-object.js:10:18:10:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
43+
| response-object.js:11:18:11:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
44+
| response-object.js:14:18:14:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
45+
| response-object.js:17:18:17:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
46+
| response-object.js:23:18:23:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
47+
| response-object.js:26:18:26:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
48+
| response-object.js:34:18:34:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
49+
| response-object.js:38:18:38:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4150
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
4251
| tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |
4352
| tst2.js:18:12:18:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:14:9:14:9 | p | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
const express = require('express');
2+
3+
// Note: We're using using express for the taint source in order to to test 'Response'
4+
// in isolation from the more complicated http frameworks.
5+
6+
express().get('/foo', (req) => {
7+
const data = req.body; // $ Source
8+
9+
new Response(data); // $ Alert
10+
new Response(data, {}); // $ Alert
11+
new Response(data, { headers: null }); // $ Alert
12+
13+
new Response(data, { headers: { 'content-type': 'text/plain'}});
14+
new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert
15+
16+
new Response(data, { headers: { 'Content-Type': 'text/plain'}});
17+
new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ Alert
18+
19+
const headers1 = new Headers({ 'content-type': 'text/plain'});
20+
new Response(data, { headers: headers1 });
21+
22+
const headers2 = new Headers({ 'content-type': 'text/html'});
23+
new Response(data, { headers: headers2 }); // $ Alert
24+
25+
const headers3 = new Headers();
26+
new Response(data, { headers: headers3 }); // $ Alert
27+
28+
const headers4 = new Headers();
29+
headers4.set('content-type', 'text/plain');
30+
new Response(data, { headers: headers4 });
31+
32+
const headers5 = new Headers();
33+
headers5.set('content-type', 'text/html');
34+
new Response(data, { headers: headers5 }); // $ Alert
35+
36+
const headers6 = new Headers();
37+
headers6.set('unrelated-header', 'text/plain');
38+
new Response(data, { headers: headers6 }); // $ Alert
39+
});

0 commit comments

Comments
 (0)