Merge pull request #3835 from Raz0r/js/xss-protocol-sinks

Approved by erik-krogh
2025-12-17 01:03:14 +01:00 · 2020-08-03 15:40:05 +01:00
parent a4f8b19ae4 3487ec17d0
commit 8855ab8c8c
5 changed files with 146 additions and 1 deletions
--- a/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll
@@ -145,4 +145,17 @@ module ClientSideUrlRedirect {
      )
    }
  }
+
+  /**
+   * A write of an attribute which may execute JavaScript code or
+   * exfiltrate data to an attacker controlled site.
+   */
+  class AttributeWriteUrlSink extends ScriptUrlSink, DataFlow::ValueNode {
+    AttributeWriteUrlSink() {
+      exists(DomPropWriteNode pw |
+        pw.interpretsValueAsJavaScriptUrl() and
+        this = DataFlow::valueNode(pw.getRhs())
+      )
+    }
+  }
 }
--- a/javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll
@@ -90,7 +90,8 @@ class DomMethodCallExpr extends MethodCallExpr {
        attr = "formaction" or
        attr = "href" or
        attr = "src" or
-        attr = "xlink:href"
+        attr = "xlink:href" or
+        attr = "data"
      |
        getArgument(argPos - 1).getStringValue().toLowerCase() = attr
      )
@@ -116,6 +117,17 @@ class DomPropWriteNode extends Assignment {
    lhs.getPropertyName() = "innerHTML" or
    lhs.getPropertyName() = "outerHTML"
  }
+
+  /**
+   * Holds if the assigned value is interpreted as JavaScript via javascript: protocol.
+   */
+  predicate interpretsValueAsJavaScriptUrl() {
+    lhs.getPropertyName() = "action" or
+    lhs.getPropertyName() = "formaction" or
+    lhs.getPropertyName() = "href" or
+    lhs.getPropertyName() = "src" or
+    lhs.getPropertyName() = "data"
+  }
 }

 /**
--- a/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected
@@ -86,6 +86,33 @@ nodes
 | tst12.js:4:15:4:45 | urlPart ... s.value |
 | tst12.js:5:23:5:25 | loc |
 | tst12.js:5:23:5:25 | loc |
+| tst13.js:2:9:2:52 | payload |
+| tst13.js:2:19:2:35 | document.location |
+| tst13.js:2:19:2:35 | document.location |
+| tst13.js:2:19:2:42 | documen ... .search |
+| tst13.js:2:19:2:52 | documen ... bstr(1) |
+| tst13.js:4:15:4:21 | payload |
+| tst13.js:4:15:4:21 | payload |
+| tst13.js:8:21:8:27 | payload |
+| tst13.js:8:21:8:27 | payload |
+| tst13.js:12:14:12:20 | payload |
+| tst13.js:12:14:12:20 | payload |
+| tst13.js:16:17:16:23 | payload |
+| tst13.js:16:17:16:23 | payload |
+| tst13.js:20:14:20:20 | payload |
+| tst13.js:20:14:20:20 | payload |
+| tst13.js:24:14:24:20 | payload |
+| tst13.js:24:14:24:20 | payload |
+| tst13.js:28:21:28:27 | payload |
+| tst13.js:28:21:28:27 | payload |
+| tst13.js:32:17:32:23 | payload |
+| tst13.js:32:17:32:23 | payload |
+| tst13.js:36:21:36:27 | payload |
+| tst13.js:36:21:36:27 | payload |
+| tst13.js:40:15:40:21 | payload |
+| tst13.js:40:15:40:21 | payload |
+| tst13.js:44:14:44:20 | payload |
+| tst13.js:44:14:44:20 | payload |
 | tst.js:2:19:2:69 | /.*redi ... n.href) |
 | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
 | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
@@ -181,6 +208,32 @@ edges
 | tst12.js:4:15:4:25 | urlParts[0] | tst12.js:4:15:4:45 | urlPart ... s.value |
 | tst12.js:4:15:4:45 | urlPart ... s.value | tst12.js:4:9:4:45 | loc |
 | tst12.js:5:23:5:25 | loc | tst12.js:3:20:3:34 | window.location |
+| tst13.js:2:9:2:52 | payload | tst13.js:4:15:4:21 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:4:15:4:21 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:8:21:8:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:8:21:8:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:12:14:12:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:12:14:12:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:16:17:16:23 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:16:17:16:23 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:20:14:20:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:20:14:20:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:24:14:24:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:24:14:24:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:28:21:28:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:28:21:28:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:32:17:32:23 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:32:17:32:23 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:36:21:36:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:36:21:36:27 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:40:15:40:21 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:40:15:40:21 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:44:14:44:20 | payload |
+| tst13.js:2:9:2:52 | payload | tst13.js:44:14:44:20 | payload |
+| tst13.js:2:19:2:35 | document.location | tst13.js:2:19:2:42 | documen ... .search |
+| tst13.js:2:19:2:35 | document.location | tst13.js:2:19:2:42 | documen ... .search |
+| tst13.js:2:19:2:42 | documen ... .search | tst13.js:2:19:2:52 | documen ... bstr(1) |
+| tst13.js:2:19:2:52 | documen ... bstr(1) | tst13.js:2:9:2:52 | payload |
 | tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
 | tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
 | tst.js:2:47:2:63 | document.location | tst.js:2:47:2:68 | documen ... on.href |
@@ -212,5 +265,16 @@ edges
 | tst10.js:11:17:11:50 | '//foo' ... .search | tst10.js:11:27:11:43 | document.location | tst10.js:11:17:11:50 | '//foo' ... .search | Untrusted URL redirection due to $@. | tst10.js:11:27:11:43 | document.location | user-provided value |
 | tst10.js:14:17:14:56 | 'https: ... .search | tst10.js:14:33:14:49 | document.location | tst10.js:14:17:14:56 | 'https: ... .search | Untrusted URL redirection due to $@. | tst10.js:14:33:14:49 | document.location | user-provided value |
 | tst12.js:5:23:5:25 | loc | tst12.js:3:20:3:34 | window.location | tst12.js:5:23:5:25 | loc | Untrusted URL redirection due to $@. | tst12.js:3:20:3:34 | window.location | user-provided value |
+| tst13.js:4:15:4:21 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:4:15:4:21 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:8:21:8:27 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:8:21:8:27 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:12:14:12:20 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:12:14:12:20 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:16:17:16:23 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:16:17:16:23 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:20:14:20:20 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:20:14:20:20 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:24:14:24:20 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:24:14:24:20 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:28:21:28:27 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:28:21:28:27 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:32:17:32:23 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:32:17:32:23 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:36:21:36:27 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:36:21:36:27 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:40:15:40:21 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:40:15:40:21 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
+| tst13.js:44:14:44:20 | payload | tst13.js:2:19:2:35 | document.location | tst13.js:44:14:44:20 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:35 | document.location | user-provided value |
 | tst.js:2:19:2:72 | /.*redi ... ref)[1] | tst.js:2:47:2:63 | document.location | tst.js:2:19:2:72 | /.*redi ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:2:47:2:63 | document.location | user-provided value |
 | tst.js:6:20:6:59 | indirec ... ref)[1] | tst.js:6:34:6:50 | document.location | tst.js:6:20:6:59 | indirec ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:6:34:6:50 | document.location | user-provided value |
--- a/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst13.js
+++ b/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst13.js
@@ -0,0 +1,46 @@
+function foo() {
+    var payload = document.location.search.substr(1);
+    var el = document.createElement("a");
+    el.href = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("button");
+    el.formaction = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("embed");
+    el.src = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("form");
+    el.action = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("frame");
+    el.src = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("iframe");
+    el.src = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("input");
+    el.formaction = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("isindex");
+    el.action = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("isindex");
+    el.formaction = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("object");
+    el.data = payload;
+    document.body.appendChild(el); // NOT OK
+
+    var el = document.createElement("script");
+    el.src = payload;
+    document.body.appendChild(el); // NOT OK
+}
--- a/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst14.js
+++ b/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst14.js
@@ -0,0 +1,10 @@
+function foo() {
+    var url = document.location.search.substr(1);
+    var el = document.createElement("script");
+    el.src = "https://github.com/" + url;
+    document.body.appendChild(el); // OK
+
+    if (url.startsWith('https://github.com/')) {
+        window.location = url; // OK
+    }
+}