JS: Handle default 'content-type' header in Response() objects

Author: asgerf

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

@@ -45,6 +45,10 @@ private class ResponseArgumentHeaders extends Http::HeaderDefinition {
   ResponseArgumentHeaders() {
     headerNode = response.getParameter(1).getMember("headers") and
     this = headerNode.asSink()
+    or
+    not exists(response.getParameter(1).getMember("headers")) and
+    headerNode = API::root() and // just bind 'headerNode' to something
+    this = response
   }
 
   ResponseCall getResponse() { result = response }
@@ -80,9 +84,33 @@ private class ResponseArgumentHeaders extends Http::HeaderDefinition {
 
   override predicate defines(string headerName, string headerValue) {
     this.getHeaderNode(headerName).getAValueReachingSink().getStringValue() = headerValue
+    or
+    // If no 'content-type' header is defined, a default one is sent in the HTTP response.
+    not exists(this.getHeaderNode("content-type")) and
+    headerName = "content-type" and
+    headerValue = "text/plain;charset=utf-8"
   }
 
-  override string getAHeaderName() { exists(this.getHeaderNode(result)) }
+  override string getAHeaderName() { exists(this.getHeaderNode(result)) or result = "content-type" }
+
+  override Http::RouteHandler getRouteHandler() { none() }
+}
+
+private class ResponseDefaultHeader extends Http::HeaderDefinition {
+  private ResponseCall response;
+
+  ResponseDefaultHeader() {
+    this = response and not exists(ResponseArgumentHeaders h | h.getResponse() = response)
+  }
+
+  override predicate defines(string headerName, string headerValue) {
+    headerName = "content-type" and
+    headerValue = "text/plain;charset=utf-8"
+  }
+
+  override string getAHeaderName() { result = "content-type" }
+
+  ResponseCall getResponse() { result = response }
 
   override Http::RouteHandler getRouteHandler() { none() }
 }

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

@@ -50,15 +50,10 @@
 | 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 |
 | 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 |
 | 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 |
-| 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 |
-| 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 |
-| 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 |
 | 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 |
 | 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 |
 | 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 |
-| 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 |
 | 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 |
-| 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 |
 | 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 |
 | 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 |
 | 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
 | promises.js:5:36:5:42 | [post update] resolve [resolve-value] | promises.js:5:16:5:22 | resolve [Return] [resolve-value] | provenance |  |
 | promises.js:5:44:5:57 | req.query.data | promises.js:5:36:5:42 | [post update] resolve [resolve-value] | provenance |  |
 | promises.js:6:11:6:11 | x | promises.js:6:25:6:25 | x | provenance |  |
-| response-object.js:7:11:7:14 | data | response-object.js:9:18:9:21 | data | provenance |  |
-| response-object.js:7:11:7:14 | data | response-object.js:10:18:10:21 | data | provenance |  |
-| response-object.js:7:11:7:14 | data | response-object.js:11:18:11:21 | data | provenance |  |
 | response-object.js:7:11:7:14 | data | response-object.js:14:18:14:21 | data | provenance |  |
 | response-object.js:7:11:7:14 | data | response-object.js:17:18:17:21 | data | provenance |  |
 | response-object.js:7:11:7:14 | data | response-object.js:23:18:23:21 | data | provenance |  |
-| response-object.js:7:11:7:14 | data | response-object.js:26:18:26:21 | data | provenance |  |
 | response-object.js:7:11:7:14 | data | response-object.js:34:18:34:21 | data | provenance |  |
-| response-object.js:7:11:7:14 | data | response-object.js:38:18:38:21 | data | provenance |  |
 | response-object.js:7:18:7:25 | req.body | response-object.js:7:11:7:14 | data | provenance |  |
 | tst2.js:6:9:6:9 | p | tst2.js:6:9:6:9 | p | provenance |  |
 | tst2.js:6:9:6:9 | p | tst2.js:7:12:7:12 | p | provenance |  |
@@ -413,15 +403,10 @@ nodes
 | promises.js:6:25:6:25 | x | semmle.label | x |
 | response-object.js:7:11:7:14 | data | semmle.label | data |
 | response-object.js:7:18:7:25 | req.body | semmle.label | req.body |
-| response-object.js:9:18:9:21 | data | semmle.label | data |
-| response-object.js:10:18:10:21 | data | semmle.label | data |
-| response-object.js:11:18:11:21 | data | semmle.label | data |
 | response-object.js:14:18:14:21 | data | semmle.label | data |
 | response-object.js:17:18:17:21 | data | semmle.label | data |
 | response-object.js:23:18:23:21 | data | semmle.label | data |
-| response-object.js:26:18:26:21 | data | semmle.label | data |
 | response-object.js:34:18:34:21 | data | semmle.label | data |
-| response-object.js:38:18:38:21 | data | semmle.label | data |
 | tst2.js:6:9:6:9 | p | semmle.label | p |
 | tst2.js:6:9:6:9 | p | semmle.label | p |
 | tst2.js:6:12:6:15 | q: r | semmle.label | q: r |

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

@@ -48,15 +48,10 @@
 | 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 |
 | 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 |
 | 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 |
-| 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 |
-| 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 |
-| 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 |
 | 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 |
 | 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 |
 | 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 |
-| 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 |
 | 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 |
-| 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 |
 | tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
 | tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |
 | 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

@@ -1,14 +1,14 @@
 const express = require('express');
 
-// Note: We're using using express for the taint source in order to to test 'Response'
+// Note: We're using express for the taint source in order to to test 'Response'
 // in isolation from the more complicated http frameworks.
 
 express().get('/foo', (req) => {
     const data = req.body; // $ Source
 
-    new Response(data); // $ Alert
-    new Response(data, {}); // $ Alert
-    new Response(data, { headers: null }); // $ Alert
+    new Response(data);
+    new Response(data, {});
+    new Response(data, { headers: null });
 
     new Response(data, { headers: { 'content-type': 'text/plain'}});
     new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert
@@ -23,7 +23,7 @@ express().get('/foo', (req) => {
     new Response(data, { headers: headers2 }); // $ Alert
 
     const headers3 = new Headers();
-    new Response(data, { headers: headers3 }); // $ Alert
+    new Response(data, { headers: headers3 });
 
     const headers4 = new Headers();
     headers4.set('content-type', 'text/plain');
@@ -35,5 +35,9 @@ express().get('/foo', (req) => {
 
     const headers6 = new Headers();
     headers6.set('unrelated-header', 'text/plain');
-    new Response(data, { headers: headers6 }); // $ Alert
+    new Response(data, { headers: headers6 });
+
+    const headers7 = new Headers();
+    headers7.set('unrelated-header', 'text/html');
+    new Response(data, { headers: headers7 });
 });