From 9ebaac82cf4ecdeb50cb118e644b584d4837bf98 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 2 Apr 2025 13:47:18 +0200 Subject: [PATCH 1/4] JS: Add tests for Response object sink --- .../CWE-079/ReflectedXss/response-object.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js new file mode 100644 index 000000000000..7dbeb14d30d3 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js @@ -0,0 +1,39 @@ +const express = require('express'); + +// Note: We're using 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; // $ MISSING: Source + + new Response(data); // $ MISSING: Alert + new Response(data, {}); // $ MISSING: Alert + new Response(data, { headers: null }); // $ MISSING: Alert + + new Response(data, { headers: { 'content-type': 'text/plain'}}); + new Response(data, { headers: { 'content-type': 'text/html'}}); // $ MISSING: Alert + + new Response(data, { headers: { 'Content-Type': 'text/plain'}}); + new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ MISSING: Alert + + const headers1 = new Headers({ 'content-type': 'text/plain'}); + new Response(data, { headers: headers1 }); + + const headers2 = new Headers({ 'content-type': 'text/html'}); + new Response(data, { headers: headers2 }); // $ MISSING: Alert + + const headers3 = new Headers(); + new Response(data, { headers: headers3 }); // $ MISSING: Alert + + const headers4 = new Headers(); + headers4.set('content-type', 'text/plain'); + new Response(data, { headers: headers4 }); + + const headers5 = new Headers(); + headers5.set('content-type', 'text/html'); + new Response(data, { headers: headers5 }); // $ MISSING: Alert + + const headers6 = new Headers(); + headers6.set('unrelated-header', 'text/plain'); + new Response(data, { headers: headers6 }); // $ MISSING: Alert +}); From db2720ea5bad641b76c416e5a11c53193a3e1099 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 2 Apr 2025 13:50:41 +0200 Subject: [PATCH 2/4] JS: Initial model of Response --- javascript/ql/lib/javascript.qll | 1 + .../javascript/frameworks/WebResponse.qll | 96 +++++++++++++++++++ .../ReflectedXss/ReflectedXss.expected | 42 ++++++++ .../ReflectedXssWithCustomSanitizer.expected | 13 +++ .../CWE-079/ReflectedXss/response-object.js | 28 +++--- 5 files changed, 166 insertions(+), 14 deletions(-) create mode 100644 javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll diff --git a/javascript/ql/lib/javascript.qll b/javascript/ql/lib/javascript.qll index cc4d15158b90..d75eed29b8e0 100644 --- a/javascript/ql/lib/javascript.qll +++ b/javascript/ql/lib/javascript.qll @@ -136,6 +136,7 @@ import semmle.javascript.frameworks.UriLibraries import semmle.javascript.frameworks.Vue import semmle.javascript.frameworks.Vuex import semmle.javascript.frameworks.Webix +import semmle.javascript.frameworks.WebResponse import semmle.javascript.frameworks.WebSocket import semmle.javascript.frameworks.XmlParsers import semmle.javascript.frameworks.xUnit diff --git a/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll new file mode 100644 index 000000000000..613e189fcfe0 --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll @@ -0,0 +1,96 @@ +/** + * Models the `Request` and `Response` objects from the Web standards. + */ + +private import javascript + +/** Treats `Response` as an entry point for API graphs. */ +private class ResponseEntryPoint extends API::EntryPoint { + ResponseEntryPoint() { this = "global.Response" } + + override DataFlow::SourceNode getASource() { result = DataFlow::globalVarRef("Response") } +} + +/** Treats `Headers` as an entry point for API graphs. */ +private class HeadersEntryPoint extends API::EntryPoint { + HeadersEntryPoint() { this = "global.Headers" } + + override DataFlow::SourceNode getASource() { result = DataFlow::globalVarRef("Headers") } +} + +/** + * A call to the `Response` constructor. + */ +private class ResponseCall extends API::InvokeNode { + ResponseCall() { this = any(ResponseEntryPoint e).getANode().getAnInstantiation() } +} + +/** + * A call to the `Headers` constructor. + */ +private class HeadersCall extends API::InvokeNode { + HeadersCall() { this = any(HeadersEntryPoint e).getANode().getAnInstantiation() } +} + +/** + * The `headers` in `new Response(body, { headers })` + */ +private class ResponseArgumentHeaders extends Http::HeaderDefinition { + private ResponseCall response; + private API::Node headerNode; + + ResponseArgumentHeaders() { + headerNode = response.getParameter(1).getMember("headers") and + this = headerNode.asSink() + } + + ResponseCall getResponse() { result = response } + + /** + * Gets a call to `new Headers()` that is passed as the headers to this call. + */ + private HeadersCall getHeadersCall() { headerNode.refersTo(result.getReturn()) } + + /** + * Gets an object whose properties are interpreted as headers, such as `{'content-type': 'foo'}`. + */ + private API::Node getAPlainHeaderObject() { + // new Response(body, {...}) + result = headerNode + or + // new Response(body, new Headers({...})) + result = this.getHeadersCall().getParameter(0) + } + + private API::Node getHeaderNode(string headerName) { + exists(string prop | + result = this.getAPlainHeaderObject().getMember(prop) and + headerName = prop.toLowerCase() + ) + or + exists(API::CallNode append | + append = this.getHeadersCall().getReturn().getMember(["append", "set"]).getACall() and + headerName = append.getArgument(0).getStringValue().toLowerCase() and + result = append.getParameter(1) + ) + } + + override predicate defines(string headerName, string headerValue) { + this.getHeaderNode(headerName).getAValueReachingSink().getStringValue() = headerValue + } + + override string getAHeaderName() { exists(this.getHeaderNode(result)) } + + override Http::RouteHandler getRouteHandler() { none() } +} + +/** + * Data passed as the body in `new Response(body, ...)`. + */ +private class ResponseSink extends Http::ResponseSendArgument { + private ResponseCall response; + + ResponseSink() { this = response.getArgument(0) } + + override Http::RouteHandler getRouteHandler() { none() } +} diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected index d85a90e4026a..9fc3e08fdb0e 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected @@ -40,6 +40,19 @@ | 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: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 | +| 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: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 | +| 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: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 | +| 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: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 | +| 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 | @@ -149,6 +162,20 @@ 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:25 | data | response-object.js:9:18:9:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:10:18:10:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:11:18:11:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:13:18:13:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:14:18:14:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:16:18:16:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:17:18:17:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:20:18:20:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:23:18:23:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:26:18:26:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:30:18:30:21 | data | provenance | | +| response-object.js:7:11:7:25 | data | response-object.js:34:18:34:21 | data | provenance | | +| response-object.js:7:11:7:25 | 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:25 | data | provenance | | | tst2.js:6:7:6:30 | p | tst2.js:7:12:7:12 | p | provenance | | | tst2.js:6:7:6:30 | r | tst2.js:8:12:8:12 | r | provenance | | | tst2.js:6:9:6:9 | p | tst2.js:6:7:6:30 | p | provenance | | @@ -332,6 +359,21 @@ nodes | promises.js:5:44:5:57 | req.query.data | semmle.label | req.query.data | | promises.js:6:11:6:11 | x | semmle.label | x | | promises.js:6:25:6:25 | x | semmle.label | x | +| response-object.js:7:11:7:25 | 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:13:18:13:21 | data | semmle.label | data | +| response-object.js:14:18:14:21 | data | semmle.label | data | +| response-object.js:16:18:16:21 | data | semmle.label | data | +| response-object.js:17:18:17:21 | data | semmle.label | data | +| response-object.js:20:18:20: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:30:18:30: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:7:6:30 | p | semmle.label | p | | tst2.js:6:7:6:30 | r | semmle.label | r | | tst2.js:6:9:6:9 | p | semmle.label | p | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected index fb0748b3acdd..dff0741ec880 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected @@ -38,6 +38,19 @@ | 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:13:18:13: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:16:18:16: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:20:18:20: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:30:18:30: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 | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js index 7dbeb14d30d3..eaadaeaba074 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js @@ -4,36 +4,36 @@ const express = require('express'); // in isolation from the more complicated http frameworks. express().get('/foo', (req) => { - const data = req.body; // $ MISSING: Source + const data = req.body; // $ Source - new Response(data); // $ MISSING: Alert - new Response(data, {}); // $ MISSING: Alert - new Response(data, { headers: null }); // $ MISSING: Alert + new Response(data); // $ Alert + new Response(data, {}); // $ Alert + new Response(data, { headers: null }); // $ Alert - new Response(data, { headers: { 'content-type': 'text/plain'}}); - new Response(data, { headers: { 'content-type': 'text/html'}}); // $ MISSING: Alert + new Response(data, { headers: { 'content-type': 'text/plain'}}); // $ SPURIOUS: Alert + new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert - new Response(data, { headers: { 'Content-Type': 'text/plain'}}); - new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ MISSING: Alert + new Response(data, { headers: { 'Content-Type': 'text/plain'}}); // $ SPURIOUS: Alert + new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ Alert const headers1 = new Headers({ 'content-type': 'text/plain'}); - new Response(data, { headers: headers1 }); + new Response(data, { headers: headers1 }); // $ SPURIOUS: Alert const headers2 = new Headers({ 'content-type': 'text/html'}); - new Response(data, { headers: headers2 }); // $ MISSING: Alert + new Response(data, { headers: headers2 }); // $ Alert const headers3 = new Headers(); - new Response(data, { headers: headers3 }); // $ MISSING: Alert + new Response(data, { headers: headers3 }); // $ Alert const headers4 = new Headers(); headers4.set('content-type', 'text/plain'); - new Response(data, { headers: headers4 }); + new Response(data, { headers: headers4 }); // $ SPURIOUS: Alert const headers5 = new Headers(); headers5.set('content-type', 'text/html'); - new Response(data, { headers: headers5 }); // $ MISSING: Alert + new Response(data, { headers: headers5 }); // $ Alert const headers6 = new Headers(); headers6.set('unrelated-header', 'text/plain'); - new Response(data, { headers: headers6 }); // $ MISSING: Alert + new Response(data, { headers: headers6 }); // $ Alert }); From 6c33013788e7a3c551c1a8a9268471fa65fe3334 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 2 Apr 2025 13:52:55 +0200 Subject: [PATCH 3/4] 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. --- .../lib/semmle/javascript/frameworks/HTTP.qll | 6 +++++ .../javascript/frameworks/WebResponse.qll | 4 +++ .../dataflow/ReflectedXssCustomizations.qll | 26 ++++++++++++------- .../ReflectedXss/ReflectedXss.expected | 12 --------- .../ReflectedXssWithCustomSanitizer.expected | 4 --- .../CWE-079/ReflectedXss/response-object.js | 8 +++--- 6 files changed, 31 insertions(+), 29 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll b/javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll index 61770cdb9bac..2d068d6b4bf6 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll @@ -108,6 +108,12 @@ module Http { * Gets the route handler that sends this expression. */ abstract RouteHandler getRouteHandler(); + + /** + * Gets a header definition associated with this response body, if it they are provided + * by the same call. + */ + HeaderDefinition getAnAssociatedHeaderDefinition() { none() } } /** diff --git a/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll index 613e189fcfe0..0e3f93d8099b 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll @@ -93,4 +93,8 @@ private class ResponseSink extends Http::ResponseSendArgument { ResponseSink() { this = response.getArgument(0) } override Http::RouteHandler getRouteHandler() { none() } + + override ResponseArgumentHeaders getAnAssociatedHeaderDefinition() { + result.getResponse() = response + } } diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll index 6ddedd4f727b..70b2685d90d4 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/ReflectedXssCustomizations.qll @@ -32,11 +32,11 @@ module ReflectedXss { * Gets a HeaderDefinition that defines a XSS safe content-type for `send`. */ Http::HeaderDefinition getAXssSafeHeaderDefinition(Http::ResponseSendArgument send) { - exists(Http::RouteHandler h | - send.getRouteHandler() = h and - result = xssSafeContentTypeHeader(h) - | - // The HeaderDefinition affects a response sent at `send`. + isSafeContentTypeHeader(result) and + ( + result = send.getAnAssociatedHeaderDefinition() + or + result = send.getRouteHandler().getAResponseHeader("content-type") and headerAffects(result, send) ) } @@ -54,14 +54,20 @@ module ReflectedXss { ] } + private predicate isSafeContentTypeHeader(Http::HeaderDefinition header) { + header.getAHeaderName() = "content-type" and + not exists(string tp | header.defines("content-type", tp) | + tp.toLowerCase().matches(xssUnsafeContentType() + "%") + ) + } + /** + * DEPRECATED. Use `getAXssSafeHeaderDefinition` instead. * Holds if `h` may send a response with a content type that is safe for XSS. */ - Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) { + deprecated Http::HeaderDefinition xssSafeContentTypeHeader(Http::RouteHandler h) { result = h.getAResponseHeader("content-type") and - not exists(string tp | result.defines("content-type", tp) | - tp.toLowerCase().matches(xssUnsafeContentType() + "%") - ) + isSafeContentTypeHeader(result) } /** @@ -80,6 +86,8 @@ module ReflectedXss { dominatingHeader.getBasicBlock().(ReachableBasicBlock).dominates(sender.getBasicBlock()) ) ) + or + header = sender.getAnAssociatedHeaderDefinition() } bindingset[headerBlock] diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected index 9fc3e08fdb0e..75bef3e1b3b3 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected @@ -43,14 +43,10 @@ | 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: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 | | 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: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 | | 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: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 | | 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: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 | | 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 | @@ -165,14 +161,10 @@ edges | response-object.js:7:11:7:25 | data | response-object.js:9:18:9:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:10:18:10:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:11:18:11:21 | data | provenance | | -| response-object.js:7:11:7:25 | data | response-object.js:13:18:13:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:14:18:14:21 | data | provenance | | -| response-object.js:7:11:7:25 | data | response-object.js:16:18:16:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:17:18:17:21 | data | provenance | | -| response-object.js:7:11:7:25 | data | response-object.js:20:18:20:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:23:18:23:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:26:18:26:21 | data | provenance | | -| response-object.js:7:11:7:25 | data | response-object.js:30:18:30:21 | data | provenance | | | response-object.js:7:11:7:25 | data | response-object.js:34:18:34:21 | data | provenance | | | response-object.js:7:11:7:25 | 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:25 | data | provenance | | @@ -364,14 +356,10 @@ nodes | 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:13:18:13:21 | data | semmle.label | data | | response-object.js:14:18:14:21 | data | semmle.label | data | -| response-object.js:16:18:16:21 | data | semmle.label | data | | response-object.js:17:18:17:21 | data | semmle.label | data | -| response-object.js:20:18:20: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:30:18:30: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:7:6:30 | p | semmle.label | p | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected index dff0741ec880..5532af3cf116 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected @@ -41,14 +41,10 @@ | 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:13:18:13: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:16:18:16: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:20:18:20: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:30:18:30: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 | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js index eaadaeaba074..030cff467335 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/response-object.js @@ -10,14 +10,14 @@ express().get('/foo', (req) => { new Response(data, {}); // $ Alert new Response(data, { headers: null }); // $ Alert - new Response(data, { headers: { 'content-type': 'text/plain'}}); // $ SPURIOUS: Alert + new Response(data, { headers: { 'content-type': 'text/plain'}}); new Response(data, { headers: { 'content-type': 'text/html'}}); // $ Alert - new Response(data, { headers: { 'Content-Type': 'text/plain'}}); // $ SPURIOUS: Alert + new Response(data, { headers: { 'Content-Type': 'text/plain'}}); new Response(data, { headers: { 'Content-Type': 'text/html'}}); // $ Alert const headers1 = new Headers({ 'content-type': 'text/plain'}); - new Response(data, { headers: headers1 }); // $ SPURIOUS: Alert + new Response(data, { headers: headers1 }); const headers2 = new Headers({ 'content-type': 'text/html'}); new Response(data, { headers: headers2 }); // $ Alert @@ -27,7 +27,7 @@ express().get('/foo', (req) => { const headers4 = new Headers(); headers4.set('content-type', 'text/plain'); - new Response(data, { headers: headers4 }); // $ SPURIOUS: Alert + new Response(data, { headers: headers4 }); const headers5 = new Headers(); headers5.set('content-type', 'text/html'); From da7d6d33468e642c704a12faf983790dd5ed2ec7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 9 Apr 2025 11:28:21 +0200 Subject: [PATCH 4/4] JS: Change note --- javascript/ql/src/change-notes/2025-04-09-web-response.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-04-09-web-response.md diff --git a/javascript/ql/src/change-notes/2025-04-09-web-response.md b/javascript/ql/src/change-notes/2025-04-09-web-response.md new file mode 100644 index 000000000000..3afebf1b6a76 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-04-09-web-response.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* 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`.