Skip to content

Commit 22f16dd

Browse files
asgerfasgerf
authored
Merge pull request #21368 from asgerf/browser-sources
JS: Add 'browser' source kinds
2 parents d52e9bc + dfa6d20 commit 22f16dd

File tree

10 files changed

+87
-18
lines changed

10 files changed

+87
-18
lines changed

‎docs/codeql/codeql-language-guides/customizing-library-models-for-javascript.rst‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ Adds a new taint source. Most taint-tracking queries will use the new source.
406406

407407
- **type**: Name of a type from which to evaluate **path**.
408408
- **path**: Access path leading to the source.
409-
- **kind**: Kind of source to add. Currently only **remote** is used.
409+
- **kind**: Kind of source to add. See the section on source kinds for a list of supported kinds.
410410

411411
Example:
412412

@@ -553,7 +553,16 @@ Kinds
553553
Source kinds
554554
~~~~~~~~~~~~
555555

556-
See documentation below for :ref:`Threat models <threat-models-javascript>`.
556+
- **remote**: A general source of remote flow.
557+
- **browser**: A source in the browser environment that does not fit a more specific browser kind.
558+
- **browser-url-query**: A source derived from the query parameters of the browser URL, such as ``location.search``.
559+
- **browser-url-fragment**: A source derived from the fragment part of the browser URL, such as ``location.hash``.
560+
- **browser-url-path**: A source derived from the pathname of the browser URL, such as ``location.pathname``.
561+
- **browser-url**: A source derived from the browser URL, where the untrusted part is prefixed by trusted data such as the scheme and hostname.
562+
- **browser-window-name**: A source derived from the window name, such as ``window.name``.
563+
- **browser-message-event**: A source derived from cross-window message passing, such as ``event`` in ``window.onmessage = event => {...}``.
564+
565+
See also :ref:`Threat models <threat-models-javascript>`.
557566

558567
Sink kinds
559568
~~~~~~~~~~

‎javascript/ql/lib/change-notes/2026-03-11-browser-source-kinds.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added support for browser-specific source kinds (`browser`, `browser-url-query`, `browser-url-fragment`, `browser-url-path`, `browser-url`, `browser-window-name`, `browser-message-event`) that can be used in data extensions to model sources in browser environments.

‎javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,18 @@ private class RemoteFlowSourceFromMaD extends RemoteFlowSource {
3535
override string getSourceType() { result = "Remote flow" }
3636
}
3737

38+
private class ClientSideRemoteFlowSourceFromMaD extends ClientSideRemoteFlowSource {
39+
private ClientSideRemoteFlowKind kind;
40+
41+
ClientSideRemoteFlowSourceFromMaD() { ModelOutput::sourceNode(this, kind) }
42+
43+
override ClientSideRemoteFlowKind getKind() { result = kind }
44+
45+
override string getSourceType() {
46+
result = "Source node (" + this.getThreatModel() + ") [from data-extension]"
47+
}
48+
}
49+
3850
/**
3951
* A threat-model flow source originating from a data extension.
4052
*/

‎javascript/ql/lib/semmle/javascript/security/dataflow/RemoteFlowSources.qll‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -43,53 +43,65 @@ import Cached
4343

4444
/**
4545
* A type of remote flow source that is specific to the browser environment.
46+
*
47+
* The underlying string also corresponds to a source kind.
4648
*/
4749
class ClientSideRemoteFlowKind extends string {
4850
ClientSideRemoteFlowKind() {
49-
this = ["query", "fragment", "path", "url", "name", "message-event"]
51+
this =
52+
[
53+
"browser", "browser-url-query", "browser-url-fragment", "browser-url-path", "browser-url",
54+
"browser-window-name", "browser-message-event"
55+
]
5056
}
5157

5258
/**
53-
* Holds if this is the `query` kind, describing sources derived from the query parameters of the browser URL,
59+
* Holds if this is the `browser` kind, indicating a remote source in a browser context, that does not fit into one
60+
* of the more specific kinds.
61+
*/
62+
predicate isGenericBrowserSourceKind() { this = "browser" }
63+
64+
/**
65+
* Holds if this is the `browser-url-query` kind, describing sources derived from the query parameters of the browser URL,
5466
* such as `location.search`.
5567
*/
56-
predicate isQuery() { this = "query" }
68+
predicate isQuery() { this = "browser-url-query" }
5769

5870
/**
59-
* Holds if this is the `frgament` kind, describing sources derived from the fragment part of the browser URL,
71+
* Holds if this is the `browser-url-fragment` kind, describing sources derived from the fragment part of the browser URL,
6072
* such as `location.hash`.
6173
*/
62-
predicate isFragment() { this = "fragment" }
74+
predicate isFragment() { this = "browser-url-fragment" }
6375

6476
/**
65-
* Holds if this is the `path` kind, describing sources derived from the pathname of the browser URL,
77+
* Holds if this is the `browser-url-path` kind, describing sources derived from the pathname of the browser URL,
6678
* such as `location.pathname`.
6779
*/
68-
predicate isPath() { this = "path" }
80+
predicate isPath() { this = "browser-url-path" }
6981

7082
/**
71-
* Holds if this is the `url` kind, describing sources derived from the browser URL,
83+
* Holds if this is the `browser-url` kind, describing sources derived from the browser URL,
7284
* where the untrusted part of the URL is prefixed by trusted data, such as the scheme and hostname.
7385
*/
74-
predicate isUrl() { this = "url" }
86+
predicate isUrl() { this = "browser-url" }
7587

76-
/** Holds if this is the `query` or `fragment` kind. */
88+
/** Holds if this is the `browser-url-query` or `browser-url-fragment` kind. */
7789
predicate isQueryOrFragment() { this.isQuery() or this.isFragment() }
7890

79-
/** Holds if this is the `path`, `query`, or `fragment` kind. */
91+
/** Holds if this is the `browser-url-path`, `browser-url-query`, or `browser-url-fragment` kind. */
8092
predicate isPathOrQueryOrFragment() { this.isPath() or this.isQuery() or this.isFragment() }
8193

82-
/** Holds if this is the `path` or `url` kind. */
94+
/** Holds if this is the `browser-url-path` or `browser-url` kind. */
8395
predicate isPathOrUrl() { this.isPath() or this.isUrl() }
8496

85-
/** Holds if this is the `name` kind, describing sources derived from the window name, such as `window.name`. */
86-
predicate isWindowName() { this = "name" }
97+
/** Holds if this is the `browser-window-name` kind, describing sources derived from the window name, such as `window.name`. */
98+
predicate isWindowName() { this = "browser-window-name" }
8799

88100
/**
89-
* Holds if this is the `message-event` kind, describing sources derived from cross-window message passing,
101+
* Holds if this is the `browser-message-event` kind, describing sources derived from cross-window message passing,
90102
* such as `event` in `window.onmessage = event => {...}`.
91103
*/
92-
predicate isMessageEvent() { this = "message-event" }
104+
predicate isMessageEvent() { this = "browser-message-event" }
93105
}
94106

95107
/**

‎javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
| 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 |
44
| 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 |
55
| 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 |
6+
| 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 |
67
edges
78
| clientSide.js:11:11:11:15 | query | clientSide.js:12:42:12:46 | query | provenance | |
89
| clientSide.js:11:19:11:40 | window. ... .search | clientSide.js:11:19:11:53 | window. ... ring(1) | provenance | |
@@ -16,6 +17,8 @@ edges
1617
| clientSide.js:20:11:20:14 | name | clientSide.js:21:42:21:45 | name | provenance | |
1718
| clientSide.js:20:18:20:28 | window.name | clientSide.js:20:11:20:14 | name | provenance | |
1819
| clientSide.js:21:42:21:45 | name | clientSide.js:21:13:21:53 | 'https: ... + '/id' | provenance | |
20+
| clientSide.js:26:11:26:16 | custom | clientSide.js:27:13:27:18 | custom | provenance | |
21+
| clientSide.js:26:20:26:56 | require ... ource() | clientSide.js:26:11:26:16 | custom | provenance | |
1922
nodes
2023
| clientSide.js:11:11:11:15 | query | semmle.label | query |
2124
| clientSide.js:11:19:11:40 | window. ... .search | semmle.label | window. ... .search |
@@ -33,4 +36,7 @@ nodes
3336
| clientSide.js:20:18:20:28 | window.name | semmle.label | window.name |
3437
| clientSide.js:21:13:21:53 | 'https: ... + '/id' | semmle.label | 'https: ... + '/id' |
3538
| clientSide.js:21:42:21:45 | name | semmle.label | name |
39+
| clientSide.js:26:11:26:16 | custom | semmle.label | custom |
40+
| clientSide.js:26:20:26:56 | require ... ource() | semmle.label | require ... ource() |
41+
| clientSide.js:27:13:27:18 | custom | semmle.label | custom |
3642
subpaths

‎javascript/ql/test/query-tests/Security/CWE-918/ClientSideRequestForgery.ext.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: sourceModel
5+
data:
6+
- ['testlib', 'Member[getBrowserSource].ReturnValue', 'browser-url-query']
7+
- ['testlib', 'Member[getServerSource].ReturnValue', 'remote']

‎javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
| serverSide.js:143:5:143:26 | axios.g ... t.href) | serverSide.js:139:19:139:31 | req.query.url | serverSide.js:143:15:143:25 | target.href | The $@ of this request depends on a $@. | serverSide.js:143:15:143:25 | target.href | URL | serverSide.js:139:19:139:31 | req.query.url | user-provided value |
4040
| serverSide.js:145:5:145:25 | axios.g ... dedUrl) | serverSide.js:139:19:139:31 | req.query.url | serverSide.js:145:15:145:24 | encodedUrl | The $@ of this request depends on a $@. | serverSide.js:145:15:145:24 | encodedUrl | URL | serverSide.js:139:19:139:31 | req.query.url | user-provided value |
4141
| serverSide.js:147:5:147:25 | axios.g ... pedUrl) | serverSide.js:139:19:139:31 | req.query.url | serverSide.js:147:15:147:24 | escapedUrl | The $@ of this request depends on a $@. | serverSide.js:147:15:147:24 | escapedUrl | URL | serverSide.js:139:19:139:31 | req.query.url | user-provided value |
42+
| 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 |
4243
edges
4344
| 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 | |
4445
| 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
144145
| serverSide.js:146:11:146:20 | escapedUrl | serverSide.js:147:15:147:24 | escapedUrl | provenance | |
145146
| serverSide.js:146:24:146:36 | escape(input) | serverSide.js:146:11:146:20 | escapedUrl | provenance | |
146147
| serverSide.js:146:31:146:35 | input | serverSide.js:146:24:146:36 | escape(input) | provenance | |
148+
| serverSide.js:150:7:150:12 | custom | serverSide.js:151:9:151:14 | custom | provenance | |
149+
| serverSide.js:150:16:150:51 | require ... ource() | serverSide.js:150:7:150:12 | custom | provenance | |
147150
nodes
148151
| Request/app/api/proxy/route2.serverSide.ts:4:9:4:15 | { url } | semmle.label | { url } |
149152
| Request/app/api/proxy/route2.serverSide.ts:4:11:4:13 | url | semmle.label | url |
@@ -271,4 +274,7 @@ nodes
271274
| serverSide.js:146:24:146:36 | escape(input) | semmle.label | escape(input) |
272275
| serverSide.js:146:31:146:35 | input | semmle.label | input |
273276
| serverSide.js:147:15:147:24 | escapedUrl | semmle.label | escapedUrl |
277+
| serverSide.js:150:7:150:12 | custom | semmle.label | custom |
278+
| serverSide.js:150:16:150:51 | require ... ource() | semmle.label | require ... ource() |
279+
| serverSide.js:151:9:151:14 | custom | semmle.label | custom |
274280
subpaths

‎javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.ext.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: sourceModel
5+
data:
6+
- ['testlib', 'Member[getBrowserSource].ReturnValue', 'browser-url-query']
7+
- ['testlib', 'Member[getServerSource].ReturnValue', 'remote']

‎javascript/ql/test/query-tests/Security/CWE-918/clientSide.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,7 @@ export function MyComponent() {
2222
request('https://example.com/api?q=' + name);
2323

2424
request(window.location.href + '?q=123');
25+
26+
const custom = require('testlib').getBrowserSource(); // $ Source[js/client-side-request-forgery]
27+
request(custom); // $ Alert[js/client-side-request-forgery]
2528
}

‎javascript/ql/test/query-tests/Security/CWE-918/serverSide.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,6 @@ var server2 = http.createServer(function (req, res) {
146146
const escapedUrl = escape(input);
147147
axios.get(escapedUrl); // $ Alert[js/request-forgery]
148148
});
149+
150+
const custom = require('testlib').getServerSource(); // $ Source[js/request-forgery]
151+
request(custom); // $ Alert[js/request-forgery]

0 commit comments

Comments
 (0)