Skip to content

Commit cc7bf4e

Browse files
committed
JS: Handle default 'content-type' header in Response() objects
1 parent 0414555 commit cc7bf4e

File tree

4 files changed

+20
-27
lines changed

4 files changed

+20
-27
lines changed

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ private class ResponseArgumentHeaders extends Http::HeaderDefinition {
4545
ResponseArgumentHeaders() {
4646
headerNode = response.getParameter(1).getMember("headers") and
4747
this = headerNode.asSink()
48+
or
49+
not exists(response.getParameter(1).getMember("headers")) and
50+
headerNode = API::root() and // just bind 'headerNode' to something
51+
this = response
4852
}
4953

5054
ResponseCall getResponse() { result = response }
@@ -80,9 +84,14 @@ private class ResponseArgumentHeaders extends Http::HeaderDefinition {
8084

8185
override predicate defines(string headerName, string headerValue) {
8286
this.getHeaderNode(headerName).getAValueReachingSink().getStringValue() = headerValue
87+
or
88+
// If no 'content-type' header is defined, a default one is sent in the HTTP response.
89+
not exists(this.getHeaderNode("content-type")) and
90+
headerName = "content-type" and
91+
headerValue = "text/plain;charset=utf-8"
8392
}
8493

85-
override string getAHeaderName() { exists(this.getHeaderNode(result)) }
94+
override string getAHeaderName() { exists(this.getHeaderNode(result)) or result = "content-type" }
8695

8796
override Http::RouteHandler getRouteHandler() { none() }
8897
}

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,10 @@
5050
| 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 |
5151
| 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 |
5252
| 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 |
53-
| 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 |
54-
| 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 |
55-
| 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 |
5653
| 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 |
5754
| 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 |
5855
| 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 |
59-
| 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 |
6056
| 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 |
61-
| 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 |
6257
| 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 |
6358
| 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 |
6459
| 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 |
@@ -184,15 +179,10 @@ edges
184179
| promises.js:5:36:5:42 | [post update] resolve [resolve-value] | promises.js:5:16:5:22 | resolve [Return] [resolve-value] | provenance | |
185180
| promises.js:5:44:5:57 | req.query.data | promises.js:5:36:5:42 | [post update] resolve [resolve-value] | provenance | |
186181
| promises.js:6:11:6:11 | x | promises.js:6:25:6:25 | x | provenance | |
187-
| response-object.js:7:11:7:14 | data | response-object.js:9:18:9:21 | data | provenance | |
188-
| response-object.js:7:11:7:14 | data | response-object.js:10:18:10:21 | data | provenance | |
189-
| response-object.js:7:11:7:14 | data | response-object.js:11:18:11:21 | data | provenance | |
190182
| response-object.js:7:11:7:14 | data | response-object.js:14:18:14:21 | data | provenance | |
191183
| response-object.js:7:11:7:14 | data | response-object.js:17:18:17:21 | data | provenance | |
192184
| response-object.js:7:11:7:14 | data | response-object.js:23:18:23:21 | data | provenance | |
193-
| response-object.js:7:11:7:14 | data | response-object.js:26:18:26:21 | data | provenance | |
194185
| response-object.js:7:11:7:14 | data | response-object.js:34:18:34:21 | data | provenance | |
195-
| response-object.js:7:11:7:14 | data | response-object.js:38:18:38:21 | data | provenance | |
196186
| response-object.js:7:18:7:25 | req.body | response-object.js:7:11:7:14 | data | provenance | |
197187
| tst2.js:6:9:6:9 | p | tst2.js:6:9:6:9 | p | provenance | |
198188
| tst2.js:6:9:6:9 | p | tst2.js:7:12:7:12 | p | provenance | |
@@ -413,15 +403,10 @@ nodes
413403
| promises.js:6:25:6:25 | x | semmle.label | x |
414404
| response-object.js:7:11:7:14 | data | semmle.label | data |
415405
| response-object.js:7:18:7:25 | req.body | semmle.label | req.body |
416-
| response-object.js:9:18:9:21 | data | semmle.label | data |
417-
| response-object.js:10:18:10:21 | data | semmle.label | data |
418-
| response-object.js:11:18:11:21 | data | semmle.label | data |
419406
| response-object.js:14:18:14:21 | data | semmle.label | data |
420407
| response-object.js:17:18:17:21 | data | semmle.label | data |
421408
| response-object.js:23:18:23:21 | data | semmle.label | data |
422-
| response-object.js:26:18:26:21 | data | semmle.label | data |
423409
| response-object.js:34:18:34:21 | data | semmle.label | data |
424-
| response-object.js:38:18:38:21 | data | semmle.label | data |
425410
| tst2.js:6:9:6:9 | p | semmle.label | p |
426411
| tst2.js:6:9:6:9 | p | semmle.label | p |
427412
| tst2.js:6:12:6:15 | q: r | semmle.label | q: r |

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,10 @@
4848
| 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 |
4949
| 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 |
5050
| 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 |
51-
| 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 |
52-
| 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 |
53-
| 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 |
5451
| 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 |
5552
| 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 |
5653
| 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 |
57-
| 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 |
5854
| 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 |
59-
| 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 |
6055
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
6156
| tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |
6257
| 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: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
const express = require('express');
22

3-
// Note: We're using using express for the taint source in order to to test 'Response'
3+
// Note: We're using express for the taint source in order to to test 'Response'
44
// in isolation from the more complicated http frameworks.
55

66
express().get('/foo', (req) => {
77
const data = req.body; // $ Source
88

9-
new Response(data); // $ Alert
10-
new Response(data, {}); // $ Alert
11-
new Response(data, { headers: null }); // $ Alert
9+
new Response(data);
10+
new Response(data, {});
11+
new Response(data, { headers: null });
1212

1313
new Response(data, { headers: { 'content-type': 'text/plain'}});
1414
new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert
@@ -23,7 +23,7 @@ express().get('/foo', (req) => {
2323
new Response(data, { headers: headers2 }); // $ Alert
2424

2525
const headers3 = new Headers();
26-
new Response(data, { headers: headers3 }); // $ Alert
26+
new Response(data, { headers: headers3 });
2727

2828
const headers4 = new Headers();
2929
headers4.set('content-type', 'text/plain');
@@ -35,5 +35,9 @@ express().get('/foo', (req) => {
3535

3636
const headers6 = new Headers();
3737
headers6.set('unrelated-header', 'text/plain');
38-
new Response(data, { headers: headers6 }); // $ Alert
38+
new Response(data, { headers: headers6 });
39+
40+
const headers7 = new Headers();
41+
headers7.set('unrelated-header', 'text/html');
42+
new Response(data, { headers: headers7 });
3943
});

0 commit comments

Comments
 (0)