Skip to content

Commit 315b1eb

Browse files
committed
JS: Enable association with headers without needing a route handler
Previously it was not possible to associate a ResponseSendArgument with its header definitions if they did not have the same route handler. But for calls like `new Response(body, { headers })` the headers are fairly obvious whereas the route handler is unnecessarily hard to find. So we use the direct and obvious association between 'body' and 'headers' in the call.
1 parent ec0b3de commit 315b1eb

File tree

6 files changed

+31
-31
lines changed

6 files changed

+31
-31
lines changed

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

+6
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

+4
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,8 @@ private class ResponseSink extends Http::ResponseSendArgument {
9393
ResponseSink() { this = response.getArgument(0) }
9494

9595
override Http::RouteHandler getRouteHandler() { none() }
96+
97+
override ResponseArgumentHeaders getAnAssociatedHeaderDefinition() {
98+
result.getResponse() = response
99+
}
96100
}

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

+17-11
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,8 @@ 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`.
40-
headerAffects(result, send)
41-
)
35+
isSafeHeader(result) and
36+
headerAffects(result, send)
4237
}
4338

4439
/**
@@ -54,14 +49,23 @@ module ReflectedXss {
5449
]
5550
}
5651

52+
private predicate isSafeHeader(Http::HeaderDefinition def) {
53+
exists(string tp | def.defines("content-type", tp) |
54+
not tp.toLowerCase().matches(xssUnsafeContentType() + "%")
55+
)
56+
or
57+
// It sets the content-type but the value is not known
58+
def.getAHeaderName() = "content-type" and
59+
not def.defines("content-type", _)
60+
}
61+
5762
/**
63+
* DEPRECATED. Use `getAXssSafeHeaderDefinition` instead.
5864
* Holds if `h` may send a response with a content type that is safe for XSS.
5965
*/
60-
Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) {
66+
deprecated Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) {
6167
result = h.getAResponseHeader("content-type") and
62-
not exists(string tp | result.defines("content-type", tp) |
63-
tp.toLowerCase().matches(xssUnsafeContentType() + "%")
64-
)
68+
isSafeHeader(result)
6569
}
6670

6771
/**
@@ -80,6 +84,8 @@ module ReflectedXss {
8084
dominatingHeader.getBasicBlock().(ReachableBasicBlock).dominates(sender.getBasicBlock())
8185
)
8286
)
87+
or
88+
header = sender.getAnAssociatedHeaderDefinition()
8389
}
8490

8591
bindingset[headerBlock]

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

-12
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,10 @@
4343
| 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 |
4444
| 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 |
4545
| 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:13:18:13:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:13:18:13:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4746
| 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 |
48-
| response-object.js:16:18:16:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:16:18:16:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4947
| 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 |
50-
| response-object.js:20:18:20:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:20:18:20:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
5148
| 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 |
5249
| 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 |
53-
| response-object.js:30:18:30:21 | data | response-object.js:7:18:7:25 | req.body | response-object.js:30:18:30:21 | data | Cross-site scripting vulnerability due to a $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
5450
| 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 |
5551
| 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 |
5652
| 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 |
@@ -165,14 +161,10 @@ edges
165161
| response-object.js:7:11:7:25 | data | response-object.js:9:18:9:21 | data | provenance | |
166162
| response-object.js:7:11:7:25 | data | response-object.js:10:18:10:21 | data | provenance | |
167163
| response-object.js:7:11:7:25 | data | response-object.js:11:18:11:21 | data | provenance | |
168-
| response-object.js:7:11:7:25 | data | response-object.js:13:18:13:21 | data | provenance | |
169164
| response-object.js:7:11:7:25 | data | response-object.js:14:18:14:21 | data | provenance | |
170-
| response-object.js:7:11:7:25 | data | response-object.js:16:18:16:21 | data | provenance | |
171165
| response-object.js:7:11:7:25 | data | response-object.js:17:18:17:21 | data | provenance | |
172-
| response-object.js:7:11:7:25 | data | response-object.js:20:18:20:21 | data | provenance | |
173166
| response-object.js:7:11:7:25 | data | response-object.js:23:18:23:21 | data | provenance | |
174167
| response-object.js:7:11:7:25 | data | response-object.js:26:18:26:21 | data | provenance | |
175-
| response-object.js:7:11:7:25 | data | response-object.js:30:18:30:21 | data | provenance | |
176168
| response-object.js:7:11:7:25 | data | response-object.js:34:18:34:21 | data | provenance | |
177169
| response-object.js:7:11:7:25 | data | response-object.js:38:18:38:21 | data | provenance | |
178170
| response-object.js:7:18:7:25 | req.body | response-object.js:7:11:7:25 | data | provenance | |
@@ -364,14 +356,10 @@ nodes
364356
| response-object.js:9:18:9:21 | data | semmle.label | data |
365357
| response-object.js:10:18:10:21 | data | semmle.label | data |
366358
| response-object.js:11:18:11:21 | data | semmle.label | data |
367-
| response-object.js:13:18:13:21 | data | semmle.label | data |
368359
| response-object.js:14:18:14:21 | data | semmle.label | data |
369-
| response-object.js:16:18:16:21 | data | semmle.label | data |
370360
| response-object.js:17:18:17:21 | data | semmle.label | data |
371-
| response-object.js:20:18:20:21 | data | semmle.label | data |
372361
| response-object.js:23:18:23:21 | data | semmle.label | data |
373362
| response-object.js:26:18:26:21 | data | semmle.label | data |
374-
| response-object.js:30:18:30:21 | data | semmle.label | data |
375363
| response-object.js:34:18:34:21 | data | semmle.label | data |
376364
| response-object.js:38:18:38:21 | data | semmle.label | data |
377365
| tst2.js:6:7:6:30 | p | semmle.label | p |

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

-4
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,10 @@
4141
| 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 |
4242
| 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 |
4343
| 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:13:18:13:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4544
| 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 |
46-
| response-object.js:16:18:16:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4745
| 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 |
48-
| response-object.js:20:18:20:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
4946
| 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 |
5047
| 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 |
51-
| response-object.js:30:18:30:21 | data | Cross-site scripting vulnerability due to $@. | response-object.js:7:18:7:25 | req.body | user-provided value |
5248
| 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 |
5349
| 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 |
5450
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ express().get('/foo', (req) => {
1010
new Response(data, {}); // $ Alert
1111
new Response(data, { headers: null }); // $ Alert
1212

13-
new Response(data, { headers: { 'content-type': 'text/plain'}}); // $ SPURIOUS: Alert
13+
new Response(data, { headers: { 'content-type': 'text/plain'}});
1414
new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert
1515

16-
new Response(data, { headers: { 'Content-Type': 'text/plain'}}); // $ SPURIOUS: Alert
16+
new Response(data, { headers: { 'Content-Type': 'text/plain'}});
1717
new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ Alert
1818

1919
const headers1 = new Headers({ 'content-type': 'text/plain'});
20-
new Response(data, { headers: headers1 }); // $ SPURIOUS: Alert
20+
new Response(data, { headers: headers1 });
2121

2222
const headers2 = new Headers({ 'content-type': 'text/html'});
2323
new Response(data, { headers: headers2 }); // $ Alert
@@ -27,7 +27,7 @@ express().get('/foo', (req) => {
2727

2828
const headers4 = new Headers();
2929
headers4.set('content-type', 'text/plain');
30-
new Response(data, { headers: headers4 }); // $ SPURIOUS: Alert
30+
new Response(data, { headers: headers4 });
3131

3232
const headers5 = new Headers();
3333
headers5.set('content-type', 'text/html');

0 commit comments

Comments
 (0)