Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS: Add sinks for calls to 'new Response()' #19200

Merged
merged 4 commits into from
Apr 10, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions javascript/ql/lib/javascript.qll
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/HTTP.qll
Original file line number Diff line number Diff line change
@@ -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() }
}

/**
100 changes: 100 additions & 0 deletions javascript/ql/lib/semmle/javascript/frameworks/WebResponse.qll
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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]
4 changes: 4 additions & 0 deletions javascript/ql/src/change-notes/2025-04-09-web-response.md
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -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 |
Original file line number Diff line number Diff line change
@@ -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
});