Skip to content

Commit f7f0547

Browse files
adityasharadadityasharad
authored
Merge pull request #4375 from adityasharad/javascript/client-side-url-redirect-regexp
JavaScript: Track taint through RegExp.prototype.exec for URL redirection
2 parents e555b6b + e712d16 commit f7f0547

File tree

3 files changed

+65
-1
lines changed

3 files changed

+65
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ module ClientSideUrlRedirect {
6565
or
6666
exists(MethodCallExpr mce |
6767
queryAccess.asExpr() = mce and
68-
mce = any(RegExpLiteral re).flow().(DataFlow::SourceNode).getAMethodCall("exec").asExpr() and
68+
mce = any(DataFlow::RegExpCreationNode re).getAMethodCall("exec").asExpr() and
6969
nd.asExpr() = mce.getArgument(0)
7070
)
7171
}

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,30 @@ nodes
133133
| tst.js:6:34:6:50 | document.location |
134134
| tst.js:6:34:6:50 | document.location |
135135
| tst.js:6:34:6:55 | documen ... on.href |
136+
| tst.js:10:19:10:81 | new Reg ... n.href) |
137+
| tst.js:10:19:10:84 | new Reg ... ref)[1] |
138+
| tst.js:10:19:10:84 | new Reg ... ref)[1] |
139+
| tst.js:10:59:10:75 | document.location |
140+
| tst.js:10:59:10:75 | document.location |
141+
| tst.js:10:59:10:80 | documen ... on.href |
142+
| tst.js:14:20:14:56 | indirec ... n.href) |
143+
| tst.js:14:20:14:59 | indirec ... ref)[1] |
144+
| tst.js:14:20:14:59 | indirec ... ref)[1] |
145+
| tst.js:14:34:14:50 | document.location |
146+
| tst.js:14:34:14:50 | document.location |
147+
| tst.js:14:34:14:55 | documen ... on.href |
148+
| tst.js:18:19:18:81 | new Reg ... n.href) |
149+
| tst.js:18:19:18:84 | new Reg ... ref)[1] |
150+
| tst.js:18:19:18:84 | new Reg ... ref)[1] |
151+
| tst.js:18:59:18:75 | document.location |
152+
| tst.js:18:59:18:75 | document.location |
153+
| tst.js:18:59:18:80 | documen ... on.href |
154+
| tst.js:22:20:22:56 | indirec ... n.href) |
155+
| tst.js:22:20:22:59 | indirec ... ref)[1] |
156+
| tst.js:22:20:22:59 | indirec ... ref)[1] |
157+
| tst.js:22:34:22:50 | document.location |
158+
| tst.js:22:34:22:50 | document.location |
159+
| tst.js:22:34:22:55 | documen ... on.href |
136160
edges
137161
| sanitizer.js:2:9:2:25 | url | sanitizer.js:4:27:4:29 | url |
138162
| sanitizer.js:2:9:2:25 | url | sanitizer.js:4:27:4:29 | url |
@@ -260,6 +284,26 @@ edges
260284
| tst.js:6:34:6:50 | document.location | tst.js:6:34:6:55 | documen ... on.href |
261285
| tst.js:6:34:6:50 | document.location | tst.js:6:34:6:55 | documen ... on.href |
262286
| tst.js:6:34:6:55 | documen ... on.href | tst.js:6:20:6:56 | indirec ... n.href) |
287+
| tst.js:10:19:10:81 | new Reg ... n.href) | tst.js:10:19:10:84 | new Reg ... ref)[1] |
288+
| tst.js:10:19:10:81 | new Reg ... n.href) | tst.js:10:19:10:84 | new Reg ... ref)[1] |
289+
| tst.js:10:59:10:75 | document.location | tst.js:10:59:10:80 | documen ... on.href |
290+
| tst.js:10:59:10:75 | document.location | tst.js:10:59:10:80 | documen ... on.href |
291+
| tst.js:10:59:10:80 | documen ... on.href | tst.js:10:19:10:81 | new Reg ... n.href) |
292+
| tst.js:14:20:14:56 | indirec ... n.href) | tst.js:14:20:14:59 | indirec ... ref)[1] |
293+
| tst.js:14:20:14:56 | indirec ... n.href) | tst.js:14:20:14:59 | indirec ... ref)[1] |
294+
| tst.js:14:34:14:50 | document.location | tst.js:14:34:14:55 | documen ... on.href |
295+
| tst.js:14:34:14:50 | document.location | tst.js:14:34:14:55 | documen ... on.href |
296+
| tst.js:14:34:14:55 | documen ... on.href | tst.js:14:20:14:56 | indirec ... n.href) |
297+
| tst.js:18:19:18:81 | new Reg ... n.href) | tst.js:18:19:18:84 | new Reg ... ref)[1] |
298+
| tst.js:18:19:18:81 | new Reg ... n.href) | tst.js:18:19:18:84 | new Reg ... ref)[1] |
299+
| tst.js:18:59:18:75 | document.location | tst.js:18:59:18:80 | documen ... on.href |
300+
| tst.js:18:59:18:75 | document.location | tst.js:18:59:18:80 | documen ... on.href |
301+
| tst.js:18:59:18:80 | documen ... on.href | tst.js:18:19:18:81 | new Reg ... n.href) |
302+
| tst.js:22:20:22:56 | indirec ... n.href) | tst.js:22:20:22:59 | indirec ... ref)[1] |
303+
| tst.js:22:20:22:56 | indirec ... n.href) | tst.js:22:20:22:59 | indirec ... ref)[1] |
304+
| tst.js:22:34:22:50 | document.location | tst.js:22:34:22:55 | documen ... on.href |
305+
| tst.js:22:34:22:50 | document.location | tst.js:22:34:22:55 | documen ... on.href |
306+
| tst.js:22:34:22:55 | documen ... on.href | tst.js:22:20:22:56 | indirec ... n.href) |
263307
#select
264308
| sanitizer.js:4:27:4:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:4:27:4:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
265309
| sanitizer.js:16:27:16:29 | url | sanitizer.js:2:15:2:25 | window.name | sanitizer.js:16:27:16:29 | url | Untrusted URL redirection due to $@. | sanitizer.js:2:15:2:25 | window.name | user-provided value |
@@ -296,3 +340,7 @@ edges
296340
| 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 |
297341
| 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 |
298342
| 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 |
343+
| tst.js:10:19:10:84 | new Reg ... ref)[1] | tst.js:10:59:10:75 | document.location | tst.js:10:19:10:84 | new Reg ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:10:59:10:75 | document.location | user-provided value |
344+
| tst.js:14:20:14:59 | indirec ... ref)[1] | tst.js:14:34:14:50 | document.location | tst.js:14:20:14:59 | indirec ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:14:34:14:50 | document.location | user-provided value |
345+
| tst.js:18:19:18:84 | new Reg ... ref)[1] | tst.js:18:59:18:75 | document.location | tst.js:18:19:18:84 | new Reg ... ref)[1] | Untrusted URL redirection due to $@. | tst.js:18:59:18:75 | document.location | user-provided value |
346+
| 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 |

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,19 @@ window.location = /.*redirect=([^&]*).*/.exec(document.location.href)[1];
55
var indirect = /.*redirect=([^&]*).*/;
66
window.location = indirect.exec(document.location.href)[1];
77
});
8+
9+
// NOT OK
10+
window.location = new RegExp('.*redirect=([^&]*).*').exec(document.location.href)[1];
11+
12+
(function(){
13+
var indirect = new RegExp('.*redirect=([^&]*).*')
14+
window.location = indirect.exec(document.location.href)[1];
15+
});
16+
17+
// NOT OK
18+
window.location = new RegExp(/.*redirect=([^&]*).*/).exec(document.location.href)[1];
19+
20+
(function(){
21+
var indirect = new RegExp(/.*redirect=([^&]*).*/)
22+
window.location = indirect.exec(document.location.href)[1];
23+
});

0 commit comments

Comments
 (0)