JS: Block flow into window.location

Author: asgerf

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

@@ -846,6 +846,14 @@ predicate clearsContent(Node n, ContentSet c) {
   // We implement this rule by clearing any captured-content before storing into another captured-content.
   VariableCaptureOutput::storeStep(getClosureNode(n), _, _) and
   c = MkAnyCapturedContent()
+  or
+  // Block flow into the "window.location" property, as any assignment/mutation to this causes a page load and stops execution.
+  // The use of clearsContent here ensures we also block assignments like `window.location.href = ...`
+  exists(DataFlow::PropRef ref |
+    ref = DataFlow::globalObjectRef().getAPropertyReference("location") and
+    n = ref.getBase().getPostUpdateNode() and
+    c = ContentSet::property("location")
+  )
 }
 
 /**

javascript/ql/test/library-tests/InterProceduralFlow/global.js

@@ -9,11 +9,11 @@ function g(x) {
 let sink1 = g(source1);
 let sink2 = g(source2);
 
-document.location = source1; // should not flow to `global2.js` in spite of assignment
+document.someProp = source1; // should not flow to `global2.js` in spite of assignment
                              // `document = {}` in `fake-document.js`
 
-window.location = source1;
+window.someProp = source1;
 let win = window;
-let sink3 = window.location;
-let sink4 = win.location;
-let sink5 = location;
+let sink3 = window.someProp;
+let sink4 = win.someProp;
+let sink5 = someProp;

javascript/ql/test/library-tests/InterProceduralFlow/global2.js

@@ -1,2 +1,2 @@
 let remote_sink = source1;
-let other_remote_sink = document.location;
+let other_remote_sink = document.someProp;