From 821cc0e875550262e6d55a8e8398b9f0b89e2acf Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 13 Mar 2026 14:58:04 +0100 Subject: [PATCH] JS: Address PR review comments - Fix misplaced semicolons in test files (was inside comment, moved before it) - Update QLdoc comments to reference new browser source kind names - Update docs to list browser source kinds and fix outdated 'only remote' note Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../customizing-library-models-for-javascript.rst | 13 +++++++++++-- .../security/dataflow/RemoteFlowSources.qll | 8 ++++---- .../test/query-tests/Security/CWE-918/clientSide.js | 2 +- .../test/query-tests/Security/CWE-918/serverSide.js | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/docs/codeql/codeql-language-guides/customizing-library-models-for-javascript.rst b/docs/codeql/codeql-language-guides/customizing-library-models-for-javascript.rst index 413471be885..a0c2e916281 100644 --- a/docs/codeql/codeql-language-guides/customizing-library-models-for-javascript.rst +++ b/docs/codeql/codeql-language-guides/customizing-library-models-for-javascript.rst @@ -406,7 +406,7 @@ Adds a new taint source. Most taint-tracking queries will use the new source. - **type**: Name of a type from which to evaluate **path**. - **path**: Access path leading to the source. -- **kind**: Kind of source to add. Currently only **remote** is used. +- **kind**: Kind of source to add. See the section on :ref:`source kinds ` for supported values. Example: @@ -553,7 +553,16 @@ Kinds Source kinds ~~~~~~~~~~~~ -See documentation below for :ref:`Threat models `. +- **remote**: A general source of remote flow. +- **browser**: A source in the browser environment that does not fit a more specific browser kind. +- **browser-url-query**: A source derived from the query parameters of the browser URL, such as ``location.search``. +- **browser-url-fragment**: A source derived from the fragment part of the browser URL, such as ``location.hash``. +- **browser-url-path**: A source derived from the pathname of the browser URL, such as ``location.pathname``. +- **browser-url**: A source derived from the browser URL, where the untrusted part is prefixed by trusted data such as the scheme and hostname. +- **browser-window-name**: A source derived from the window name, such as ``window.name``. +- **browser-message-event**: A source derived from cross-window message passing, such as ``event`` in ``window.onmessage = event => {...}``. + +See also :ref:`Threat models `. Sink kinds ~~~~~~~~~~ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll index 5b1424fb86e..8791382180a 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll @@ -85,16 +85,16 @@ class ClientSideRemoteFlowKind extends string { */ predicate isUrl() { this = "browser-url" } - /** Holds if this is the `query` or `fragment` kind. */ + /** Holds if this is the `browser-url-query` or `browser-url-fragment` kind. */ predicate isQueryOrFragment() { this.isQuery() or this.isFragment() } - /** Holds if this is the `path`, `query`, or `fragment` kind. */ + /** Holds if this is the `browser-url-path`, `browser-url-query`, or `browser-url-fragment` kind. */ predicate isPathOrQueryOrFragment() { this.isPath() or this.isQuery() or this.isFragment() } - /** Holds if this is the `path` or `url` kind. */ + /** Holds if this is the `browser-url-path` or `browser-url` kind. */ 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`. */ + /** Holds if this is the `browser-window-name` kind, describing sources derived from the window name, such as `window.name`. */ predicate isWindowName() { this = "browser-window-name" } /** 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 1651fb01f44..a8e29602f1f 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js +++ b/javascript/ql/test/query-tests/Security/CWE-918/clientSide.js @@ -24,5 +24,5 @@ export function MyComponent() { 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]; + 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 7cf16ccb1ed..c359312738b 100644 --- a/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js +++ b/javascript/ql/test/query-tests/Security/CWE-918/serverSide.js @@ -148,4 +148,4 @@ var server2 = http.createServer(function (req, res) { }); const custom = require('testlib').getServerSource(); // $ Source[js/request-forgery] -request(custom) // $ Alert[js/request-forgery]; +request(custom); // $ Alert[js/request-forgery]