Skip to content

Commit 608a031

Browse files
committed
add location reads from the history libary as client-side remote flow
1 parent e543c6c commit 608a031

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,3 +272,30 @@ private class WindowLocationFlowSource extends ClientSideRemoteFlowSource {
272272

273273
override ClientSideRemoteFlowKind getKind() { result = kind }
274274
}
275+
276+
/**
277+
* A user-controlled location value read from the [history](http://npmjs.org/package/history) library.
278+
*/
279+
private class HistoryLibaryRemoteFlow extends ClientSideRemoteFlowSource {
280+
ClientSideRemoteFlowKind kind;
281+
282+
HistoryLibaryRemoteFlow() {
283+
exists(API::Node loc |
284+
loc =
285+
API::moduleImport("history")
286+
.getMember(["createBrowserHistory", "createHashHistory"])
287+
.getReturn()
288+
.getMember("location")
289+
|
290+
this = loc.getMember("hash").getAnImmediateUse() and kind.isFragment()
291+
or
292+
this = loc.getMember("pathname").getAnImmediateUse() and kind.isPath()
293+
or
294+
this = loc.getMember("search").getAnImmediateUse() and kind.isQuery()
295+
)
296+
}
297+
298+
override string getSourceType() { result = "Window location" }
299+
300+
override ClientSideRemoteFlowKind getKind() { result = kind }
301+
}

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/ClientSideUrlRedirect.expected

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ nodes
135135
| tst13.js:59:19:59:52 | documen ... bstr(1) |
136136
| tst13.js:61:18:61:24 | payload |
137137
| tst13.js:61:18:61:24 | payload |
138+
| tst13.js:65:9:65:49 | payload |
139+
| tst13.js:65:19:65:39 | history ... on.hash |
140+
| tst13.js:65:19:65:39 | history ... on.hash |
141+
| tst13.js:65:19:65:49 | history ... bstr(1) |
142+
| tst13.js:67:21:67:27 | payload |
143+
| tst13.js:67:21:67:27 | payload |
138144
| tst.js:2:19:2:69 | /.*redi ... n.href) |
139145
| tst.js:2:19:2:72 | /.*redi ... ref)[1] |
140146
| tst.js:2:19:2:72 | /.*redi ... ref)[1] |
@@ -317,6 +323,11 @@ edges
317323
| tst13.js:59:19:59:42 | documen ... .search | tst13.js:59:19:59:52 | documen ... bstr(1) |
318324
| tst13.js:59:19:59:42 | documen ... .search | tst13.js:59:19:59:52 | documen ... bstr(1) |
319325
| tst13.js:59:19:59:52 | documen ... bstr(1) | tst13.js:59:9:59:52 | payload |
326+
| tst13.js:65:9:65:49 | payload | tst13.js:67:21:67:27 | payload |
327+
| tst13.js:65:9:65:49 | payload | tst13.js:67:21:67:27 | payload |
328+
| tst13.js:65:19:65:39 | history ... on.hash | tst13.js:65:19:65:49 | history ... bstr(1) |
329+
| tst13.js:65:19:65:39 | history ... on.hash | tst13.js:65:19:65:49 | history ... bstr(1) |
330+
| tst13.js:65:19:65:49 | history ... bstr(1) | tst13.js:65:9:65:49 | payload |
320331
| tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
321332
| tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
322333
| tst.js:2:47:2:63 | document.location | tst.js:2:47:2:68 | documen ... on.href |
@@ -409,6 +420,7 @@ edges
409420
| tst13.js:50:23:50:23 | e | tst13.js:49:32:49:32 | e | tst13.js:50:23:50:23 | e | Untrusted URL redirection due to $@. | tst13.js:49:32:49:32 | e | user-provided value |
410421
| tst13.js:53:28:53:28 | e | tst13.js:52:34:52:34 | e | tst13.js:53:28:53:28 | e | Untrusted URL redirection due to $@. | tst13.js:52:34:52:34 | e | user-provided value |
411422
| tst13.js:61:18:61:24 | payload | tst13.js:59:19:59:42 | documen ... .search | tst13.js:61:18:61:24 | payload | Untrusted URL redirection due to $@. | tst13.js:59:19:59:42 | documen ... .search | user-provided value |
423+
| tst13.js:67:21:67:27 | payload | tst13.js:65:19:65:39 | history ... on.hash | tst13.js:67:21:67:27 | payload | Untrusted URL redirection due to $@. | tst13.js:65:19:65:39 | history ... on.hash | user-provided value |
412424
| 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 |
413425
| tst.js:2:19:2:72 | /.*redi ... ref)[1] | tst.js:2:47:2:68 | documen ... on.href | tst.js:2:19:2:72 | /.*redi ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:2:47:2:68 | documen ... on.href | user-provided value |
414426
| 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 |

javascript/ql/test/query-tests/Security/CWE-601/ClientSideUrlRedirect/tst13.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,15 @@ function foo() {
5454
}
5555
})();
5656

57-
const history = require('history').createBrowserHistory();
5857
function bar() {
58+
const history = require('history').createBrowserHistory();
5959
var payload = document.location.search.substr(1);
6060

6161
history.push(payload); // NOT OK
62+
}
63+
function baz() {
64+
const history = require('history').createHashHistory();
65+
var payload = history.location.hash.substr(1);
66+
67+
history.replace(payload); // NOT OK
6268
}

0 commit comments

Comments
 (0)