Skip to content

Commit 00c8387

Browse files
author
Esben Sparre Andreasen
committed
JS: model Koa redirects
1 parent 298dbe1 commit 00c8387

File tree

4 files changed

+69
-0
lines changed

4 files changed

+69
-0
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,4 +291,20 @@ module Koa {
291291

292292
override RouteHandler getRouteHandler() { result = rh }
293293
}
294+
295+
/**
296+
* An invocation of the `redirect` method of an HTTP response object.
297+
*/
298+
private class RedirectInvocation extends HTTP::RedirectInvocation, MethodCallExpr {
299+
RouteHandler rh;
300+
301+
RedirectInvocation() {
302+
this.(MethodCallExpr).calls(rh.getAResponseOrContextExpr(), "redirect")
303+
}
304+
305+
override Expr getUrlArgument() { result = getArgument(0) }
306+
307+
override RouteHandler getRouteHandler() { result = rh }
308+
}
309+
294310
}

‎javascript/ql/test/library-tests/frameworks/koa/tests.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,5 @@ test_ContextExpr
161161
| src/koa.js:43:2:43:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
162162
| src/koa.js:44:2:44:4 | ctx | src/koa.js:30:10:45:1 | async c ... url);\\n} |
163163
test_RedirectInvocation
164+
| src/koa.js:43:2:43:18 | ctx.redirect(url) | src/koa.js:43:15:43:17 | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |
165+
| src/koa.js:44:2:44:27 | ctx.res ... ct(url) | src/koa.js:44:24:44:26 | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ nodes
2222
| express.js:135:23:135:37 | req.params.user |
2323
| express.js:136:16:136:36 | 'u' + r ... ms.user |
2424
| express.js:136:22:136:36 | req.params.user |
25+
| koa.js:6:6:6:27 | url |
26+
| koa.js:6:12:6:27 | ctx.query.target |
27+
| koa.js:7:15:7:17 | url |
28+
| koa.js:8:15:8:26 | `${url}${x}` |
29+
| koa.js:8:18:8:20 | url |
30+
| koa.js:14:16:14:18 | url |
2531
| node.js:6:7:6:52 | target |
2632
| node.js:6:16:6:39 | url.par ... , true) |
2733
| node.js:6:16:6:45 | url.par ... ).query |
@@ -60,6 +66,24 @@ edges
6066
| express.js:134:22:134:36 | req.params.user | express.js:134:16:134:36 | '/' + r ... ms.user |
6167
| express.js:135:23:135:37 | req.params.user | express.js:135:16:135:37 | '//' + ... ms.user |
6268
| express.js:136:22:136:36 | req.params.user | express.js:136:16:136:36 | 'u' + r ... ms.user |
69+
| koa.js:6:6:6:27 | url | koa.js:7:15:7:17 | url |
70+
| koa.js:6:6:6:27 | url | koa.js:8:18:8:20 | url |
71+
| koa.js:6:6:6:27 | url | koa.js:10:40:10:42 | url |
72+
| koa.js:6:6:6:27 | url | koa.js:10:40:10:42 | url |
73+
| koa.js:6:6:6:27 | url | koa.js:10:51:10:51 | url |
74+
| koa.js:6:6:6:27 | url | koa.js:11:6:11:8 | url |
75+
| koa.js:6:6:6:27 | url | koa.js:14:16:14:18 | url |
76+
| koa.js:6:12:6:27 | ctx.query.target | koa.js:6:6:6:27 | url |
77+
| koa.js:8:18:8:20 | url | koa.js:8:15:8:26 | `${url}${x}` |
78+
| koa.js:10:40:10:42 | url | koa.js:10:51:10:51 | url |
79+
| koa.js:10:40:10:42 | url | koa.js:10:51:10:51 | url |
80+
| koa.js:10:40:10:42 | url | koa.js:11:6:11:8 | url |
81+
| koa.js:10:40:10:42 | url | koa.js:11:6:11:8 | url |
82+
| koa.js:10:40:10:42 | url | koa.js:14:16:14:18 | url |
83+
| koa.js:10:40:10:42 | url | koa.js:14:16:14:18 | url |
84+
| koa.js:10:51:10:51 | url | koa.js:11:6:11:8 | url |
85+
| koa.js:10:51:10:51 | url | koa.js:14:16:14:18 | url |
86+
| koa.js:11:6:11:8 | url | koa.js:14:16:14:18 | url |
6387
| node.js:6:7:6:52 | target | node.js:7:34:7:39 | target |
6488
| node.js:6:16:6:39 | url.par ... , true) | node.js:6:16:6:45 | url.par ... ).query |
6589
| node.js:6:16:6:45 | url.par ... ).query | node.js:6:16:6:52 | url.par ... .target |
@@ -95,6 +119,9 @@ edges
95119
| express.js:134:16:134:36 | '/' + r ... ms.user | express.js:134:22:134:36 | req.params.user | express.js:134:16:134:36 | '/' + r ... ms.user | Untrusted URL redirection due to $@. | express.js:134:22:134:36 | req.params.user | user-provided value |
96120
| express.js:135:16:135:37 | '//' + ... ms.user | express.js:135:23:135:37 | req.params.user | express.js:135:16:135:37 | '//' + ... ms.user | Untrusted URL redirection due to $@. | express.js:135:23:135:37 | req.params.user | user-provided value |
97121
| express.js:136:16:136:36 | 'u' + r ... ms.user | express.js:136:22:136:36 | req.params.user | express.js:136:16:136:36 | 'u' + r ... ms.user | Untrusted URL redirection due to $@. | express.js:136:22:136:36 | req.params.user | user-provided value |
122+
| koa.js:7:15:7:17 | url | koa.js:6:12:6:27 | ctx.query.target | koa.js:7:15:7:17 | url | Untrusted URL redirection due to $@. | koa.js:6:12:6:27 | ctx.query.target | user-provided value |
123+
| koa.js:8:15:8:26 | `${url}${x}` | koa.js:6:12:6:27 | ctx.query.target | koa.js:8:15:8:26 | `${url}${x}` | Untrusted URL redirection due to $@. | koa.js:6:12:6:27 | ctx.query.target | user-provided value |
124+
| koa.js:14:16:14:18 | url | koa.js:6:12:6:27 | ctx.query.target | koa.js:14:16:14:18 | url | Untrusted URL redirection due to $@. | koa.js:6:12:6:27 | ctx.query.target | user-provided value |
98125
| node.js:7:34:7:39 | target | node.js:6:26:6:32 | req.url | node.js:7:34:7:39 | target | Untrusted URL redirection due to $@. | node.js:6:26:6:32 | req.url | user-provided value |
99126
| node.js:15:34:15:45 | '/' + target | node.js:11:26:11:32 | req.url | node.js:15:34:15:45 | '/' + target | Untrusted URL redirection due to $@. | node.js:11:26:11:32 | req.url | user-provided value |
100127
| node.js:32:34:32:55 | target ... =" + me | node.js:29:26:29:32 | req.url | node.js:32:34:32:55 | target ... =" + me | Untrusted URL redirection due to $@. | node.js:29:26:29:32 | req.url | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/koa.js‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
const Koa = require('koa');
2+
const url = require('url');
3+
const app = new Koa();
4+
5+
app.use(async ctx => {
6+
var url = ctx.query.target;
7+
ctx.redirect(url); // NOT OK
8+
ctx.redirect(`${url}${x}`); // NOT OK
9+
10+
var isCrossDomainRedirect = url.parse(url || '', false, true).hostname;
11+
if(!url || isCrossDomainRedirect) {
12+
ctx.redirect('/'); // OK
13+
} else {
14+
ctx.redirect(url); // NOT OK
15+
}
16+
17+
if(!url || isCrossDomainRedirect || ! url.match(VALID)) {
18+
ctx.redirect('/'); // OK
19+
} else {
20+
ctx.redirect(url); // OK
21+
}
22+
});
23+
24+
app.listen(3000);

0 commit comments

Comments
 (0)