track window object to where .location is read

erik-krogh · erik-krogh · commit dfd63e5d5afa · 2021-04-28T18:52:00.000+02:00
diff --git a/javascript/ql/src/semmle/javascript/DOM.qll b/javascript/ql/src/semmle/javascript/DOM.qll
@@ -469,6 +469,9 @@ module DOM {
     t.start() and
     result = locationSource()
     or
+    t.startInProp("location") and
+    result = [DataFlow::globalObjectRef(), documentSource()]
+    or
     exists(DataFlow::TypeTracker t2 | result = locationRef(t2).track(t2, t))
   }
 
diff --git a/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected b/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected
@@ -171,6 +171,11 @@ nodes
 | tst.js:22:34:22:50 | document.location |
 | tst.js:22:34:22:55 | documen ... on.href |
 | tst.js:22:34:22:55 | documen ... on.href |
+| tst.js:26:22:26:79 | new Reg ... n.href) |
+| tst.js:26:22:26:82 | new Reg ... ref)[1] |
+| tst.js:26:22:26:82 | new Reg ... ref)[1] |
+| tst.js:26:62:26:78 | win.location.href |
+| tst.js:26:62:26:78 | win.location.href |
 | typed.ts:4:13:4:36 | params |
 | typed.ts:4:22:4:36 | location.search |
 | typed.ts:4:22:4:36 | location.search |
@@ -337,6 +342,10 @@ edges
 | tst.js:22:34:22:50 | document.location | tst.js:22:34:22:55 | documen ... on.href |
 | tst.js:22:34:22:55 | documen ... on.href | tst.js:22:20:22:56 | indirec ... n.href) |
 | tst.js:22:34:22:55 | documen ... on.href | tst.js:22:20:22:56 | indirec ... n.href) |
+| tst.js:26:22:26:79 | new Reg ... n.href) | tst.js:26:22:26:82 | new Reg ... ref)[1] |
+| tst.js:26:22:26:79 | new Reg ... n.href) | tst.js:26:22:26:82 | new Reg ... ref)[1] |
+| tst.js:26:62:26:78 | win.location.href | tst.js:26:22:26:79 | new Reg ... n.href) |
+| tst.js:26:62:26:78 | win.location.href | tst.js:26:22:26:79 | new Reg ... n.href) |
 | typed.ts:4:13:4:36 | params | typed.ts:5:25:5:30 | params |
 | typed.ts:4:22:4:36 | location.search | typed.ts:4:13:4:36 | params |
 | typed.ts:4:22:4:36 | location.search | typed.ts:4:13:4:36 | params |
@@ -400,5 +409,6 @@ edges
 | tst.js:18:19:18:84 | new Reg ... ref)[1] | tst.js:18:59:18:80 | documen ... on.href | tst.js:18:19:18:84 | new Reg ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:18:59:18:80 | documen ... on.href | user-provided value |
 | tst.js:22:20:22:59 | indirec ... ref)[1] | tst.js:22:34:22:50 | document.location | tst.js:22:20:22:59 | indirec ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:22:34:22:50 | document.location | user-provided value |
 | tst.js:22:20:22:59 | indirec ... ref)[1] | tst.js:22:34:22:55 | documen ... on.href | tst.js:22:20:22:59 | indirec ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:22:34:22:55 | documen ... on.href | user-provided value |
+| tst.js:26:22:26:82 | new Reg ... ref)[1] | tst.js:26:62:26:78 | win.location.href | tst.js:26:22:26:82 | new Reg ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:26:62:26:78 | win.location.href | user-provided value |
 | typed.ts:8:33:8:43 | redirectUri | typed.ts:4:22:4:36 | location.search | typed.ts:8:33:8:43 | redirectUri | Untrusted URL redirection due to $@. | typed.ts:4:22:4:36 | location.search | user-provided value |
 | typed.ts:29:33:29:43 | redirectUri | typed.ts:25:25:25:34 | loc.search | typed.ts:29:33:29:43 | redirectUri | Untrusted URL redirection due to $@. | typed.ts:25:25:25:34 | loc.search | user-provided value |
diff --git a/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst.js b/javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst.js
@@ -21,3 +21,9 @@ window.location = new RegExp(/.*redirect=([^&]*).*/).exec(document.location.href
 	var indirect = new RegExp(/.*redirect=([^&]*).*/)
 	window.location = indirect.exec(document.location.href)[1];
 });
+
+function foo(win) {
+	win.location.assign(new RegExp(/.*redirect=([^&]*).*/).exec(win.location.href)[1]); // NOT OK
+}
+
+foo(window);

Original file line number	Diff line number	Diff line change
`@@ -469,6 +469,9 @@ module DOM {`
`469`	`469`	`t.start() and`
`470`	`470`	`result = locationSource()`
`471`	`471`	`or`
	`472`	`+ t.startInProp("location") and`
	`473`	`+ result = [DataFlow::globalObjectRef(), documentSource()]`
	`474`	`+ or`
`472`	`475`	`exists(DataFlow::TypeTracker t2 \| result = locationRef(t2).track(t2, t))`
`473`	`476`	`}`
`474`	`477`