From 3ae319c2d2e37c4be05fb47df26bb6412b099479 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 25 Feb 2026 09:09:51 +0100 Subject: [PATCH 1/3] JS: Add browser source kinds --- .../frameworks/data/ModelsAsData.qll | 12 +++++++ .../security/dataflow/RemoteFlowSources.qll | 36 ++++++++++++------- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index e4adff2a9ac3..391f2ea1b337 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -35,6 +35,18 @@ private class RemoteFlowSourceFromMaD extends RemoteFlowSource { override string getSourceType() { result = "Remote flow" } } +private class ClientSideRemoteFlowSourceFromMaD extends ClientSideRemoteFlowSource { + private ClientSideRemoteFlowKind kind; + + ClientSideRemoteFlowSourceFromMaD() { ModelOutput::sourceNode(this, kind) } + + override ClientSideRemoteFlowKind getKind() { result = kind } + + override string getSourceType() { + result = "Source node (" + this.getThreatModel() + ") [from data-extension]" + } +} + /** * A threat-model flow source originating from a data extension. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index 9f4975e605ae..5b1424fb86e3 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -43,35 +43,47 @@ import Cached /** * A type of remote flow source that is specific to the browser environment. + * + * The underlying string also corresponds to a source kind. */ class ClientSideRemoteFlowKind extends string { ClientSideRemoteFlowKind() { - this = ["query", "fragment", "path", "url", "name", "message-event"] + this = + [ + "browser", "browser-url-query", "browser-url-fragment", "browser-url-path", "browser-url", + "browser-window-name", "browser-message-event" + ] } /** - * Holds if this is the `query` kind, describing sources derived from the query parameters of the browser URL, + * Holds if this is the `browser` kind, indicating a remote source in a browser context, that does not fit into one + * of the more specific kinds. + */ + predicate isGenericBrowserSourceKind() { this = "browser" } + + /** + * Holds if this is the `browser-url-query` kind, describing sources derived from the query parameters of the browser URL, * such as `location.search`. */ - predicate isQuery() { this = "query" } + predicate isQuery() { this = "browser-url-query" } /** - * Holds if this is the `frgament` kind, describing sources derived from the fragment part of the browser URL, + * Holds if this is the `browser-url-fragment` kind, describing sources derived from the fragment part of the browser URL, * such as `location.hash`. */ - predicate isFragment() { this = "fragment" } + predicate isFragment() { this = "browser-url-fragment" } /** - * Holds if this is the `path` kind, describing sources derived from the pathname of the browser URL, + * Holds if this is the `browser-url-path` kind, describing sources derived from the pathname of the browser URL, * such as `location.pathname`. */ - predicate isPath() { this = "path" } + predicate isPath() { this = "browser-url-path" } /** - * Holds if this is the `url` kind, describing sources derived from the browser URL, + * Holds if this is the `browser-url` kind, describing sources derived from the browser URL, * where the untrusted part of the URL is prefixed by trusted data, such as the scheme and hostname. */ - predicate isUrl() { this = "url" } + predicate isUrl() { this = "browser-url" } /** Holds if this is the `query` or `fragment` kind. */ predicate isQueryOrFragment() { this.isQuery() or this.isFragment() } @@ -83,13 +95,13 @@ class ClientSideRemoteFlowKind extends string { predicate isPathOrUrl() { this.isPath() or this.isUrl() } /** Holds if this is the `name` kind, describing sources derived from the window name, such as `window.name`. */ - predicate isWindowName() { this = "name" } + predicate isWindowName() { this = "browser-window-name" } /** - * Holds if this is the `message-event` kind, describing sources derived from cross-window message passing, + * Holds if this is the `browser-message-event` kind, describing sources derived from cross-window message passing, * such as `event` in `window.onmessage = event => {...}`. */ - predicate isMessageEvent() { this = "message-event" } + predicate isMessageEvent() { this = "browser-message-event" } } /** From 66d3fdad44aeec078781ec6a2189db1cdbae533e Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 25 Feb 2026 09:41:38 +0100 Subject: [PATCH 2/3] JS: Add tests in request forgery queries --- .../Security/CWE-918/ClientSideRequestForgery.expected | 6 ++++++ .../Security/CWE-918/ClientSideRequestForgery.ext.yml | 7 +++++++ .../query-tests/Security/CWE-918/RequestForgery.expected | 6 ++++++ .../query-tests/Security/CWE-918/RequestForgery.ext.yml | 7 +++++++ .../ql/test/query-tests/Security/CWE-918/clientSide.js | 3 +++ .../ql/test/query-tests/Security/CWE-918/serverSide.js | 3 +++ 6 files changed, 32 insertions(+) create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.ext.yml create mode 100644 javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.ext.yml diff --git a/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.expected index 1d6b8781db75..aed2cabe358a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.expected @@ -3,6 +3,7 @@ | clientSide.js:14:5:14:64 | request ... search) | clientSide.js:14:42:14:63 | window. ... .search | clientSide.js:14:13:14:63 | 'https: ... .search | The $@ of this request depends on a $@. | clientSide.js:14:13:14:63 | 'https: ... .search | URL | clientSide.js:14:42:14:63 | window. ... .search | user-provided value | | clientSide.js:17:5:17:58 | request ... '/id') | clientSide.js:16:22:16:41 | window.location.hash | clientSide.js:17:13:17:57 | 'https: ... + '/id' | The $@ of this request depends on a $@. | clientSide.js:17:13:17:57 | 'https: ... + '/id' | URL | clientSide.js:16:22:16:41 | window.location.hash | user-provided value | | clientSide.js:21:5:21:54 | request ... '/id') | clientSide.js:20:18:20:28 | window.name | clientSide.js:21:13:21:53 | 'https: ... + '/id' | The $@ of this request depends on a $@. | clientSide.js:21:13:21:53 | 'https: ... + '/id' | URL | clientSide.js:20:18:20:28 | window.name | user-provided value | +| clientSide.js:27:5:27:19 | request(custom) | clientSide.js:26:20:26:56 | require ... ource() | clientSide.js:27:13:27:18 | custom | The $@ of this request depends on a $@. | clientSide.js:27:13:27:18 | custom | URL | clientSide.js:26:20:26:56 | require ... ource() | user-provided value | edges | clientSide.js:11:11:11:15 | query | clientSide.js:12:42:12:46 | query | provenance | | | clientSide.js:11:19:11:40 | window. ... .search | clientSide.js:11:19:11:53 | window. ... ring(1) | provenance | | @@ -16,6 +17,8 @@ edges | clientSide.js:20:11:20:14 | name | clientSide.js:21:42:21:45 | name | provenance | | | clientSide.js:20:18:20:28 | window.name | clientSide.js:20:11:20:14 | name | provenance | | | clientSide.js:21:42:21:45 | name | clientSide.js:21:13:21:53 | 'https: ... + '/id' | provenance | | +| clientSide.js:26:11:26:16 | custom | clientSide.js:27:13:27:18 | custom | provenance | | +| clientSide.js:26:20:26:56 | require ... ource() | clientSide.js:26:11:26:16 | custom | provenance | | nodes | clientSide.js:11:11:11:15 | query | semmle.label | query | | clientSide.js:11:19:11:40 | window. ... .search | semmle.label | window. ... .search | @@ -33,4 +36,7 @@ nodes | clientSide.js:20:18:20:28 | window.name | semmle.label | window.name | | clientSide.js:21:13:21:53 | 'https: ... + '/id' | semmle.label | 'https: ... + '/id' | | clientSide.js:21:42:21:45 | name | semmle.label | name | +| clientSide.js:26:11:26:16 | custom | semmle.label | custom | +| clientSide.js:26:20:26:56 | require ... ource() | semmle.label | require ... ource() | +| clientSide.js:27:13:27:18 | custom | semmle.label | custom | subpaths diff --git a/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.ext.yml b/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.ext.yml new file mode 100644 index 000000000000..950186d58df7 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.ext.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: sourceModel + data: + - ['testlib', 'Member[getBrowserSource].ReturnValue', 'browser-url-query'] + - ['testlib', 'Member[getServerSource].ReturnValue', 'remote'] diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected index a91a6348dfa6..c5db682a1e5f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected @@ -39,6 +39,7 @@ | serverSide.js:143:3:143:24 | axios.g ... t.href) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:143:13:143:23 | target.href | The $@ of this request depends on a $@. | serverSide.js:143:13:143:23 | target.href | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value | | serverSide.js:145:3:145:23 | axios.g ... dedUrl) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:145:13:145:22 | encodedUrl | The $@ of this request depends on a $@. | serverSide.js:145:13:145:22 | encodedUrl | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value | | serverSide.js:147:3:147:23 | axios.g ... pedUrl) | serverSide.js:139:17:139:29 | req.query.url | serverSide.js:147:13:147:22 | escapedUrl | The $@ of this request depends on a $@. | serverSide.js:147:13:147:22 | escapedUrl | URL | serverSide.js:139:17:139:29 | req.query.url | user-provided value | +| serverSide.js:151:1:151:15 | request(custom) | serverSide.js:150:16:150:51 | require ... ource() | serverSide.js:151:9:151:14 | custom | The $@ of this request depends on a $@. | serverSide.js:151:9:151:14 | custom | URL | serverSide.js:150:16:150:51 | require ... ource() | user-provided value | edges | Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | Request/app/api/proxy/route2.serverSide.ts:4:11:4:13 | url | provenance | | | Request/app/api/proxy/route2.serverSide.ts:4:11:4:13 | url | Request/app/api/proxy/route2.serverSide.ts:5:27:5:29 | url | provenance | | @@ -144,6 +145,8 @@ edges | serverSide.js:146:9:146:18 | escapedUrl | serverSide.js:147:13:147:22 | escapedUrl | provenance | | | serverSide.js:146:22:146:34 | escape(input) | serverSide.js:146:9:146:18 | escapedUrl | provenance | | | serverSide.js:146:29:146:33 | input | serverSide.js:146:22:146:34 | escape(input) | provenance | | +| serverSide.js:150:7:150:12 | custom | serverSide.js:151:9:151:14 | custom | provenance | | +| serverSide.js:150:16:150:51 | require ... ource() | serverSide.js:150:7:150:12 | custom | provenance | | nodes | Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | semmle.label | { url } | | Request/app/api/proxy/route2.serverSide.ts:4:11:4:13 | url | semmle.label | url | @@ -271,4 +274,7 @@ nodes | serverSide.js:146:22:146:34 | escape(input) | semmle.label | escape(input) | | serverSide.js:146:29:146:33 | input | semmle.label | input | | serverSide.js:147:13:147:22 | escapedUrl | semmle.label | escapedUrl | +| serverSide.js:150:7:150:12 | custom | semmle.label | custom | +| serverSide.js:150:16:150:51 | require ... ource() | semmle.label | require ... ource() | +| serverSide.js:151:9:151:14 | custom | semmle.label | custom | subpaths diff --git a/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.ext.yml b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.ext.yml new file mode 100644 index 000000000000..950186d58df7 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.ext.yml @@ -0,0 +1,7 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: sourceModel + data: + - ['testlib', 'Member[getBrowserSource].ReturnValue', 'browser-url-query'] + - ['testlib', 'Member[getServerSource].ReturnValue', 'remote'] diff --git a/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js b/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js index aa4174cd9ab7..1651fb01f44d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js +++ b/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js @@ -22,4 +22,7 @@ export function MyComponent() { request('https://example.com/api?q=' + name); request(window.location.href + '?q=123'); + + const custom = require('testlib').getBrowserSource(); // $ Source[js/client-side-request-forgery] + request(custom) // $ Alert[js/client-side-request-forgery]; } diff --git a/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js b/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js index c578b268e400..62d99cde01ff 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js +++ b/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js @@ -146,3 +146,6 @@ var server2 = http.createServer(function(req, res) { const escapedUrl = escape(input); axios.get(escapedUrl); // $Alert[js/request-forgery] }); + +const custom = require('testlib').getServerSource(); // $ Source[js/request-forgery] +request(custom) // $ Alert[js/request-forgery]; From 2a41ff2719dbff80208ce336563e8f0cc17300fc Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 25 Feb 2026 09:41:49 +0100 Subject: [PATCH 3/3] JS: Fix some whitespace --- .../ql/test/query-tests/Security/CWE-918/serverSide.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js b/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js index 62d99cde01ff..88e429817b4c 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js +++ b/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js @@ -59,11 +59,11 @@ var server = http.createServer(async function(req, res) { var client = await CDP(options); client.Page.navigate({url: tainted}); // $ Alert[js/request-forgery] - + CDP(options).catch((ignored) => {}).then((client) => { client.Page.navigate({url: tainted}); // $ Alert[js/request-forgery] }) - + CDP(options, (client) => { client.Page.navigate({url: tainted}); // $ Alert[js/request-forgery] }); @@ -127,15 +127,15 @@ var server2 = http.createServer(function(req, res) { url: tainted // $ Sink[js/request-forgery] }) // $ Alert[js/request-forgery] - var myUrl = `${something}/bla/${tainted}`; + var myUrl = `${something}/bla/${tainted}`; axios.get(myUrl); // $ Alert[js/request-forgery] - var myEncodedUrl = `${something}/bla/${encodeURIComponent(tainted)}`; + var myEncodedUrl = `${something}/bla/${encodeURIComponent(tainted)}`; axios.get(myEncodedUrl); }) var server2 = http.createServer(function(req, res) { - const { URL } = require('url'); + const { URL } = require('url'); const input = req.query.url; // $Source[js/request-forgery] const target = new URL(input); axios.get(target.toString()); // $Alert[js/request-forgery]