JS: Add missing store step in Xss query

Author: asgerf

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

@@ -28,6 +28,17 @@ module DomBasedXss {
       guard instanceof SanitizerGuard
     }
 
+    override predicate isAdditionalStoreStep(
+      DataFlow::Node pred, DataFlow::SourceNode succ, string prop
+    ) {
+      exists(DataFlow::PropRead read |
+        pred = read.getBase() and
+        succ = read and
+        read.getPropertyName() = "hash" and
+        prop = urlSuffixPseudoProperty()
+      )
+    }
+
     override predicate isAdditionalLoadStoreStep(
       DataFlow::Node pred, DataFlow::Node succ, string predProp, string succProp
     ) {

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

@@ -453,6 +453,12 @@ nodes
 | tst.js:414:19:414:31 | target.taint8 |
 | tst.js:415:18:415:30 | target.taint8 |
 | tst.js:415:18:415:30 | target.taint8 |
+| tst.js:422:7:422:46 | payload |
+| tst.js:422:17:422:31 | window.location |
+| tst.js:422:17:422:31 | window.location |
+| tst.js:422:17:422:46 | window. ... bstr(1) |
+| tst.js:423:18:423:24 | payload |
+| tst.js:423:18:423:24 | payload |
 | typeahead.js:20:13:20:45 | target |
 | typeahead.js:20:22:20:38 | document.location |
 | typeahead.js:20:22:20:38 | document.location |
@@ -882,6 +888,11 @@ edges
 | tst.js:414:19:414:31 | target.taint8 | tst.js:414:19:414:31 | target.taint8 |
 | tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
 | tst.js:414:19:414:31 | target.taint8 | tst.js:415:18:415:30 | target.taint8 |
+| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
+| tst.js:422:7:422:46 | payload | tst.js:423:18:423:24 | payload |
+| 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 |
 | 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 |
@@ -1009,6 +1020,7 @@ edges
 | tst.js:403:18:403:30 | target.taint5 | tst.js:387:16:387:32 | document.location | tst.js:403:18:403:30 | target.taint5 | Cross-site scripting vulnerability due to $@. | tst.js:387:16:387:32 | document.location | user-provided value |
 | 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 |
 | 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

@@ -418,3 +418,7 @@ function test() {
   $('myId').html(target.taint9); // OK
 }
 
+function hash2() {
+  var payload = window.location.hash.substr(1);
+  document.write(payload); // NOT OK
+}