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/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 new file mode 100644 index 000000000000..0e3f93d8099b --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll @@ -0,0 +1,100 @@ +/** + * 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() } + + 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/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`. 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..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 @@ -40,6 +40,15 @@ | 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 | @@ -149,6 +158,16 @@ 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:14:18:14: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: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: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 +351,17 @@ 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: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: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..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 @@ -38,6 +38,15 @@ | 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 | 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..030cff467335 --- /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; // $ Source + + 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'}}); // $ 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 }); + + const headers2 = new Headers({ 'content-type': 'text/html'}); + new Response(data, { headers: headers2 }); // $ Alert + + const headers3 = new Headers(); + new Response(data, { headers: headers3 }); // $ 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 }); // $ Alert + + const headers6 = new Headers(); + headers6.set('unrelated-header', 'text/plain'); + new Response(data, { headers: headers6 }); // $ Alert +});