Skip to content

Commit 6bdd7df

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6002 from erik-krogh/history
Approved by asgerf
2 parents a02f96d + b1d7c61 commit 6bdd7df

File tree

6 files changed

+125
-0
lines changed

6 files changed

+125
-0
lines changed

‎javascript/change-notes/2021-06-03-history.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* Taint sources and sinks from the [history](https://npmjs.com/package/history) library are now recognized.
3+
Affected packages are
4+
[history](https://www.npmjs.com/package/history)

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ import semmle.javascript.frameworks.FormParsers
9393
import semmle.javascript.frameworks.jQuery
9494
import semmle.javascript.frameworks.JWT
9595
import semmle.javascript.frameworks.Handlebars
96+
import semmle.javascript.frameworks.History
9697
import semmle.javascript.frameworks.Immutable
9798
import semmle.javascript.frameworks.LazyCache
9899
import semmle.javascript.frameworks.LodashUnderscore

‎javascript/ql/src/semmle/javascript/frameworks/History.qll

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/** Provides classes and predicates modelling aspects of the [`history`](https://npmjs.org/package/history) library. */
2+
3+
import javascript
4+
5+
/** Provides classes modelling the [`history`](https://npmjs.org/package/history) library. */
6+
module History {
7+
/** The global variable `HistoryLibrary` as an entry point for API graphs. */
8+
private class HistoryGlobalEntry extends API::EntryPoint {
9+
HistoryGlobalEntry() { this = "HistoryLibrary" }
10+
11+
override DataFlow::SourceNode getAUse() { result = DataFlow::globalVarRef("HistoryLibrary") }
12+
13+
override DataFlow::Node getARhs() { none() }
14+
}
15+
16+
/**
17+
* Gets a reference to the [`history`](https://npmjs.org/package/history) library.
18+
*/
19+
private API::Node history() {
20+
result = [API::moduleImport("history"), API::root().getASuccessor(any(HistoryGlobalEntry h))]
21+
}
22+
23+
/**
24+
* Gets a browser history instance.
25+
* This history instance uses the native browser history API.
26+
*/
27+
API::Node getBrowserHistory() { result = history().getMember("createBrowserHistory").getReturn() }
28+
29+
/**
30+
* Gets a hash history instance.
31+
* This history instance only manipulates the URL hash, which cannot cause XSS.
32+
*/
33+
API::Node getHashHistory() { result = history().getMember("createHashHistory").getReturn() }
34+
35+
/**
36+
* A user-controlled location value read from the [history](http://npmjs.org/package/history) library.
37+
*/
38+
private class HistoryLibaryRemoteFlow extends ClientSideRemoteFlowSource {
39+
ClientSideRemoteFlowKind kind;
40+
41+
HistoryLibaryRemoteFlow() {
42+
exists(API::Node loc | loc = [getBrowserHistory(), getHashHistory()].getMember("location") |
43+
this = loc.getMember("hash").getAnImmediateUse() and kind.isFragment()
44+
or
45+
this = loc.getMember("pathname").getAnImmediateUse() and kind.isPath()
46+
or
47+
this = loc.getMember("search").getAnImmediateUse() and kind.isQuery()
48+
)
49+
}
50+
51+
override string getSourceType() { result = "Window location" }
52+
53+
override ClientSideRemoteFlowKind getKind() { result = kind }
54+
}
55+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,15 @@ module ClientSideUrlRedirect {
191191
}
192192
}
193193

194+
/**
195+
* A write to the location using the [history](https://npmjs.com/package/history) library
196+
*/
197+
class HistoryWriteUrlSink extends ScriptUrlSink {
198+
HistoryWriteUrlSink() {
199+
this = History::getBrowserHistory().getMember(["push", "replace"]).getACall().getArgument(0)
200+
}
201+
}
202+
194203
/**
195204
* A call to change the current url with a Next.js router.
196205
*/

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,24 @@ nodes
129129
| tst13.js:52:34:52:34 | e |
130130
| tst13.js:53:28:53:28 | e |
131131
| tst13.js:53:28:53:28 | e |
132+
| tst13.js:59:9:59:52 | payload |
133+
| tst13.js:59:19:59:42 | documen ... .search |
134+
| tst13.js:59:19:59:42 | documen ... .search |
135+
| tst13.js:59:19:59:52 | documen ... bstr(1) |
136+
| tst13.js:61:18:61:24 | payload |
137+
| 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 |
144+
| tst13.js:72:9:72:49 | payload |
145+
| tst13.js:72:19:72:39 | history ... on.hash |
146+
| tst13.js:72:19:72:39 | history ... on.hash |
147+
| tst13.js:72:19:72:49 | history ... bstr(1) |
148+
| tst13.js:74:21:74:27 | payload |
149+
| tst13.js:74:21:74:27 | payload |
132150
| tst.js:2:19:2:69 | /.*redi ... n.href) |
133151
| tst.js:2:19:2:72 | /.*redi ... ref)[1] |
134152
| tst.js:2:19:2:72 | /.*redi ... ref)[1] |
@@ -306,6 +324,21 @@ edges
306324
| tst13.js:52:34:52:34 | e | tst13.js:53:28:53:28 | e |
307325
| tst13.js:52:34:52:34 | e | tst13.js:53:28:53:28 | e |
308326
| tst13.js:52:34:52:34 | e | tst13.js:53:28:53:28 | e |
327+
| tst13.js:59:9:59:52 | payload | tst13.js:61:18:61:24 | payload |
328+
| tst13.js:59:9:59:52 | payload | tst13.js:61:18:61:24 | payload |
329+
| tst13.js:59:19:59:42 | documen ... .search | tst13.js:59:19:59:52 | documen ... bstr(1) |
330+
| tst13.js:59:19:59:42 | documen ... .search | tst13.js:59:19:59:52 | documen ... bstr(1) |
331+
| tst13.js:59:19:59:52 | documen ... bstr(1) | tst13.js:59:9:59:52 | payload |
332+
| tst13.js:65:9:65:49 | payload | tst13.js:67:21:67:27 | payload |
333+
| tst13.js:65:9:65:49 | payload | tst13.js:67:21:67:27 | payload |
334+
| tst13.js:65:19:65:39 | history ... on.hash | tst13.js:65:19:65:49 | history ... bstr(1) |
335+
| tst13.js:65:19:65:39 | history ... on.hash | tst13.js:65:19:65:49 | history ... bstr(1) |
336+
| tst13.js:65:19:65:49 | history ... bstr(1) | tst13.js:65:9:65:49 | payload |
337+
| tst13.js:72:9:72:49 | payload | tst13.js:74:21:74:27 | payload |
338+
| tst13.js:72:9:72:49 | payload | tst13.js:74:21:74:27 | payload |
339+
| tst13.js:72:19:72:39 | history ... on.hash | tst13.js:72:19:72:49 | history ... bstr(1) |
340+
| tst13.js:72:19:72:39 | history ... on.hash | tst13.js:72:19:72:49 | history ... bstr(1) |
341+
| tst13.js:72:19:72:49 | history ... bstr(1) | tst13.js:72:9:72:49 | payload |
309342
| tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
310343
| tst.js:2:19:2:69 | /.*redi ... n.href) | tst.js:2:19:2:72 | /.*redi ... ref)[1] |
311344
| tst.js:2:47:2:63 | document.location | tst.js:2:47:2:68 | documen ... on.href |
@@ -397,6 +430,9 @@ edges
397430
| tst13.js:44:14:44:20 | payload | tst13.js:2:19:2:42 | documen ... .search | tst13.js:44:14:44:20 | payload | Untrusted URL redirection due to $@. | tst13.js:2:19:2:42 | documen ... .search | user-provided value |
398431
| 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 |
399432
| 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 |
433+
| 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 |
434+
| 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 |
435+
| tst13.js:74:21:74:27 | payload | tst13.js:72:19:72:39 | history ... on.hash | tst13.js:74:21:74:27 | payload | Untrusted URL redirection due to $@. | tst13.js:72:19:72:39 | history ... on.hash | user-provided value |
400436
| 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 |
401437
| 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 |
402438
| 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: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,23 @@ function foo() {
5353
self.importScripts(e); // NOT OK
5454
}
5555
})();
56+
57+
function bar() {
58+
const history = require('history').createBrowserHistory();
59+
var payload = document.location.search.substr(1);
60+
61+
history.push(payload); // NOT OK
62+
}
63+
function baz() {
64+
const history = require('history').createBrowserHistory();
65+
var payload = history.location.hash.substr(1);
66+
67+
history.replace(payload); // NOT OK
68+
}
69+
70+
function quz() {
71+
const history = HistoryLibrary.createBrowserHistory();
72+
var payload = history.location.hash.substr(1);
73+
74+
history.replace(payload); // NOT OK
75+
}

0 commit comments

Comments
 (0)