hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

change SimpleParameter to Parameter in the express model to support destructuring parameters

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-09-26 21:31:06 +02:00
parent fc84286b56
commit 664342dd0f
3 changed files with 23 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
javascript/ql/src/semmle/javascript/frameworks/Express.qll
View File

@@ -195,7 +195,7 @@ module Express {
PassportRouteHandler() { this = any(PassportRouteSetup setup).getARouteHandler() }
override SimpleParameter getRouteHandlerParameter(string kind) {
override Parameter getRouteHandlerParameter(string kind) {
kind = "request" and
result = astNode.getParameter(0)
}
@@ -329,17 +329,17 @@ module Express {
*
* `kind` is one of: "error", "request", "response", "next", or "parameter".
*/
abstract SimpleParameter getRouteHandlerParameter(string kind);
abstract Parameter getRouteHandlerParameter(string kind);
/**
* Gets the parameter of the route handler that contains the request object.
*/
SimpleParameter getRequestParameter() { result = getRouteHandlerParameter("request") }
Parameter getRequestParameter() { result = getRouteHandlerParameter("request") }
/**
* Gets the parameter of the route handler that contains the response object.
*/
SimpleParameter getResponseParameter() { result = getRouteHandlerParameter("response") }
Parameter getResponseParameter() { result = getRouteHandlerParameter("response") }
/**
* Gets a request body access of this handler.
@@ -357,7 +357,7 @@ module Express {
StandardRouteHandler() { this = routeSetup.getARouteHandler() }
override SimpleParameter getRouteHandlerParameter(string kind) {
override Parameter getRouteHandlerParameter(string kind) {
if routeSetup.isParameterHandler()
then result = getRouteParameterHandlerParameter(astNode, kind)
else result = getRouteHandlerParameter(astNode, kind)
@@ -890,7 +890,7 @@ module Express {
TrackedRouteHandlerCandidateWithSetup() { this = routeSetup.getARouteHandler() }
override SimpleParameter getRouteHandlerParameter(string kind) {
override Parameter getRouteHandlerParameter(string kind) {
if routeSetup.isParameterHandler()
then result = getRouteParameterHandlerParameter(astNode, kind)
else result = getRouteHandlerParameter(astNode, kind)

10
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/ServerSideUrlRedirect.expected
View File

@@ -44,6 +44,12 @@ nodes
| express.js:136:16:136:36 | 'u' + r ... ms.user |
| express.js:136:22:136:36 | req.params.user |
| express.js:136:22:136:36 | req.params.user |
| express.js:143:16:143:28 | req.query.foo |
| express.js:143:16:143:28 | req.query.foo |
| express.js:143:16:143:28 | req.query.foo |
| express.js:146:16:146:24 | query.foo |
| express.js:146:16:146:24 | query.foo |
| express.js:146:16:146:24 | query.foo |
| koa.js:6:6:6:27 | url |
| koa.js:6:12:6:27 | ctx.query.target |
| koa.js:6:12:6:27 | ctx.query.target |
@@ -128,6 +134,8 @@ edges
| express.js:136:22:136:36 | req.params.user | 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 |
| express.js:136:22:136:36 | req.params.user | express.js:136:16:136:36 | 'u' + r ... ms.user |
| express.js:143:16:143:28 | req.query.foo | express.js:143:16:143:28 | req.query.foo |
| express.js:146:16:146:24 | query.foo | express.js:146:16:146:24 | query.foo |
| koa.js:6:6:6:27 | url | koa.js:7:15:7:17 | url |
| koa.js:6:6:6:27 | url | koa.js:7:15:7:17 | url |
| koa.js:6:6:6:27 | url | koa.js:8:18:8:20 | url |
@@ -181,6 +189,8 @@ edges
| 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 |
| 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 |
| 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 |
| express.js:143:16:143:28 | req.query.foo | express.js:143:16:143:28 | req.query.foo | express.js:143:16:143:28 | req.query.foo | Untrusted URL redirection due to $@. | express.js:143:16:143:28 | req.query.foo | user-provided value |
| express.js:146:16:146:24 | query.foo | express.js:146:16:146:24 | query.foo | express.js:146:16:146:24 | query.foo | Untrusted URL redirection due to $@. | express.js:146:16:146:24 | query.foo | user-provided value |
| 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 |
| 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 |
| 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 |

7
javascript/ql/test/query-tests/Security/CWE-601/ServerSideUrlRedirect/express.js
View File

@@ -138,3 +138,10 @@ app.get('/redirect/:user', function(req, res) {
res.redirect('/' + ('/u' + req.params.user)); // BAD - could go to //u.evil.com, but not flagged [INCONSISTENCY]
res.redirect('/u' + req.params.user); // GOOD
});
app.get("foo", (req, res) => {
res.redirect(req.query.foo); // NOT OK
});
app.get("bar", ({query}, res) => {
res.redirect(query.foo); // NOT OK
})