JS: Support .hash extraction via a few more methods

Author: asgerf

javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll

@@ -52,15 +52,29 @@ module DomBasedXss {
     }
 
     override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
-      exists(DataFlow::MethodCallNode call, string name |
-        name = "substr" or name = "substring" or name = "slice"
-      |
-        call.getMethodName() = name and
+      exists(DataFlow::MethodCallNode call |
+        call.getMethodName() = ["substr", "substring", "slice"] and
         not call.getArgument(0).getIntValue() = 0 and
         pred = call.getReceiver() and
         succ = call and
         prop = urlSuffixPseudoProperty()
       )
+      or
+      exists(DataFlow::MethodCallNode call |
+        call.getMethodName() = "exec" and pred = call.getArgument(0)
+        or
+        call.getMethodName() = "match" and pred = call.getReceiver()
+      |
+        succ = call and
+        prop = urlSuffixPseudoProperty()
+      )
+      or
+      exists(StringSplitCall split |
+        split.getSeparator() = ["#", "?"] and
+        pred = split.getBaseString() and
+        succ = split.getASubstringRead(1) and
+        prop = urlSuffixPseudoProperty()
+      )
     }
 
     override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {

javascript/ql/test/query-tests/Security/CWE-079/Xss.expected

@@ -459,6 +459,17 @@ nodes
 | tst.js:422:17:422:46 | window. ... bstr(1) |
 | tst.js:423:18:423:24 | payload |
 | tst.js:423:18:423:24 | payload |
+| tst.js:425:7:425:55 | match |
+| tst.js:425:15:425:29 | window.location |
+| tst.js:425:15:425:29 | window.location |
+| tst.js:425:15:425:55 | window. ... (\\w+)/) |
+| tst.js:427:20:427:24 | match |
+| tst.js:427:20:427:27 | match[1] |
+| tst.js:427:20:427:27 | match[1] |
+| tst.js:430:18:430:32 | window.location |
+| tst.js:430:18:430:32 | window.location |
+| tst.js:430:18:430:51 | window. ... '#')[1] |
+| tst.js:430:18:430:51 | window. ... '#')[1] |
 | typeahead.js:20:13:20:45 | target |
 | typeahead.js:20:22:20:38 | document.location |
 | typeahead.js:20:22:20:38 | document.location |
@@ -893,6 +904,16 @@ edges
 | tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
 | tst.js:422:17:422:31 | window.location | tst.js:422:17:422:46 | window. ... bstr(1) |
 | tst.js:422:17:422:46 | window. ... bstr(1) | tst.js:422:7:422:46 | payload |
+| tst.js:425:7:425:55 | match | tst.js:427:20:427:24 | match |
+| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
+| tst.js:425:15:425:29 | window.location | tst.js:425:15:425:55 | window. ... (\\w+)/) |
+| tst.js:425:15:425:55 | window. ... (\\w+)/) | tst.js:425:7:425:55 | match |
+| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
+| tst.js:427:20:427:24 | match | tst.js:427:20:427:27 | match[1] |
+| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
+| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
+| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
+| tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] |
 | typeahead.js:20:13:20:45 | target | typeahead.js:21:12:21:17 | target |
 | typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
 | typeahead.js:20:22:20:38 | document.location | typeahead.js:20:22:20:45 | documen ... .search |
@@ -1021,6 +1042,8 @@ edges
 | tst.js:412:18:412:30 | target.taint7 | tst.js:387:16:387:32 | document.location | tst.js:412:18:412:30 | target.taint7 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
 | tst.js:415:18:415:30 | target.taint8 | tst.js:387:16:387:32 | document.location | tst.js:415:18:415:30 | target.taint8 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
 | tst.js:423:18:423:24 | payload | tst.js:422:17:422:31 | window.location | tst.js:423:18:423:24 | payload | Cross-site scripting vulnerability due to $@. | tst.js:422:17:422:31 | window.location | user-provided value |
+| tst.js:427:20:427:27 | match[1] | tst.js:425:15:425:29 | window.location | tst.js:427:20:427:27 | match[1] | Cross-site scripting vulnerability due to $@. | tst.js:425:15:425:29 | window.location | user-provided value |
+| tst.js:430:18:430:51 | window. ... '#')[1] | tst.js:430:18:430:32 | window.location | tst.js:430:18:430:51 | window. ... '#')[1] | Cross-site scripting vulnerability due to $@. | tst.js:430:18:430:32 | window.location | user-provided value |
 | typeahead.js:25:18:25:20 | val | typeahead.js:20:22:20:38 | document.location | typeahead.js:25:18:25:20 | val | Cross-site scripting vulnerability due to $@. | typeahead.js:20:22:20:38 | document.location | user-provided value |
 | v-html.vue:2:8:2:23 | v-html=tainted | v-html.vue:6:42:6:58 | document.location | v-html.vue:2:8:2:23 | v-html=tainted | Cross-site scripting vulnerability due to $@. | v-html.vue:6:42:6:58 | document.location | user-provided value |
 | winjs.js:3:43:3:49 | tainted | winjs.js:2:17:2:33 | document.location | winjs.js:3:43:3:49 | tainted | Cross-site scripting vulnerability due to $@. | winjs.js:2:17:2:33 | document.location | user-provided value |

javascript/ql/test/query-tests/Security/CWE-079/tst.js

@@ -421,4 +421,11 @@ function test() {
 function hash2() {
   var payload = window.location.hash.substr(1);
   document.write(payload); // NOT OK
+
+  let match = window.location.hash.match(/hello (\w+)/);
+  if (match) {
+    document.write(match[1]); // NOT OK
+  }
+
+  document.write(window.location.hash.split('#')[1]); // NOT OK
 }