JS: Add 'browser' source kinds and corresponding threat model kinds

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.
  */

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

@@ -30,6 +30,9 @@ private module Cached {
    */
   cached
   abstract class ClientSideRemoteFlowSource extends RemoteFlowSource {
+    cached
+    override string getThreatModel() { result = this.getKind() }
+
     /** Gets a string indicating what part of the browser environment this was derived from. */
     cached
     abstract ClientSideRemoteFlowKind getKind();
@@ -43,35 +46,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 and threat model 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 +98,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" }
 }
 
 /**

javascript/ql/test/library-tests/threat-models/default/ActiveKinds.expected

@@ -1,3 +1,10 @@
+| browser |
+| browser-message-event |
+| browser-url |
+| browser-url-fragment |
+| browser-url-path |
+| browser-url-query |
+| browser-window-name |
 | default |
 | remote |
 | request |

javascript/ql/test/library-tests/threat-models/sources/TestSources.ext.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/javascript-all
+      extensible: sourceModel
+    data:
+      - ['testlib', 'Member[getBrowserSource].ReturnValue', 'browser']

javascript/ql/test/library-tests/threat-models/sources/sources.js

@@ -118,3 +118,42 @@ rl_stdin.question('<question>', handler);
 rl_stdin.on("line", (line) => { // $ threat-source=stdin
     SINK(line); // $ hasFlow
 });
+
+// ------ browser sources ------
+
+function browserSources() {
+    const loc = window.location; // $ threat-source=browser-url
+
+    // Accessing the browser URL via document.location
+    SINK(loc); // $ hasFlow
+
+    // Accessing the browser URL via location.href
+    var href = loc.href; // $ threat-source=browser-url
+    SINK(href); // $ hasFlow
+
+    // Accessing the query string via location.search
+    var search = loc.search; // $ threat-source=browser-url-query
+    SINK(search); // $ hasFlow
+
+    // Accessing the fragment via location.hash
+    var hash = loc.hash; // $ threat-source=browser-url-fragment
+    SINK(hash); // $ hasFlow
+
+    // Accessing window.name
+    var wname = window.name; // $ threat-source=browser-window-name
+    SINK(wname); // $ hasFlow
+
+    // Accessing message events via window.onmessage
+    window.onmessage = function(event) { // $ threat-source=browser-message-event
+        SINK(event); // $ hasFlow
+    };
+
+    // Accessing message events via addEventListener
+    window.addEventListener("message", function(event) { // $ threat-source=browser-message-event
+        SINK(event); // $ hasFlow
+    });
+
+    // Test custom source
+    const customSource = require('testlib').getBrowserSource(); // $ threat-source=browser
+    SINK(customSource); // $ hasFlow
+}

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

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']

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

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']

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];
 }

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]
@@ -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];

shared/threat-models/ext/threat-model-grouping.model.yml

@@ -23,6 +23,15 @@ extensions:
       - ["android-external-storage-dir", "android"]
       - ["contentprovider", "android"]
 
+      # Browser sources
+      - ["browser", "remote"]
+      - ["browser-window-name", "browser"]
+      - ["browser-message-event", "browser"]
+      - ["browser-url", "browser"]
+      - ["browser-url-query", "browser-url"]
+      - ["browser-url-path", "browser-url"]
+      - ["browser-url-fragment", "browser-url"]
+
       # Threat models that are not grouped with any other threat models.
       # (Note that all threat models are a child of "all" implicitly, and we
       # make it explicit here just to make sure all threat models are listed.)