Skip to content

Commit 6cfd790

Browse files
authored
Merge pull request github#9356 from erik-krogh/getRouting
JS: rewrite js/sensitive-get-query to use routing trees
2 parents bb93179 + 95fae81 commit 6cfd790

File tree

3 files changed

+27
-5
lines changed

3 files changed

+27
-5
lines changed

javascript/ql/src/Security/CWE-598/SensitiveGetQuery.ql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
import javascript
1515

1616
from
17-
Express::RouteSetup setup, Express::RouteHandler handler, Express::RequestInputAccess input,
17+
Routing::RouteSetup setup, Routing::RouteHandler handler, HTTP::RequestInputAccess input,
1818
SensitiveExpr sensitive
1919
where
20-
setup.getRequestMethod() = "GET" and
21-
handler = setup.getARouteHandler() and
22-
input.getRouteHandler() = handler and
20+
setup.getOwnHttpMethod() = "GET" and
21+
setup.getAChild+() = handler and
22+
input.getRouteHandler() = handler.getFunction() and
2323
input.getKind() = "parameter" and
2424
input.(DataFlow::SourceNode).flowsToExpr(sensitive) and
2525
not sensitive.getClassification() = SensitiveDataClassification::id()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
| tst.js:8:22:8:39 | req.query.password | $@ for GET requests uses query parameter as sensitive data. | tst.js:6:19:14:1 | (req, r ... serId\\n} | Route handler |
22
| tst.js:26:22:26:42 | req.par ... sword') | $@ for GET requests uses query parameter as sensitive data. | tst.js:24:20:35:1 | (req, r ... });\\n} | Route handler |
33
| tst.js:31:24:31:40 | req.param('word') | $@ for GET requests uses query parameter as sensitive data. | tst.js:24:20:35:1 | (req, r ... });\\n} | Route handler |
4+
| tst.js:39:29:39:41 | query.current | $@ for GET requests uses query parameter as sensitive data. | tst.js:37:19:43:1 | ({query ... });\\n} | Route handler |
5+
| tst.js:50:33:50:52 | req.param('current') | $@ for GET requests uses query parameter as sensitive data. | tst.js:48:12:54:5 | (req, r ... ;\\n } | Route handler |

javascript/ql/test/query-tests/Security/CWE-598/tst.js

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,24 @@ app.get("/login2", (req, res) => {
3232
checkUser(username, myPassword, (result) => {
3333
res.send(result);
3434
});
35-
});
35+
});
36+
37+
app.get("/login", ({query}, res) => {
38+
const username = query.username; // OK - usernames are fine
39+
const currentPassword = query.current; // NOT OK - password read
40+
checkUser(username, currentPassword, (result) => {
41+
res.send(result);
42+
});
43+
});
44+
45+
app.get('/rest/user/change-password', mkHandler());
46+
47+
function mkHandler() {
48+
return (req, res) => {
49+
const username = req.param('username'); // OK - usernames are fine
50+
const currentPassword = req.param('current'); // NOT OK - password read
51+
checkUser(username, currentPassword, (result) => {
52+
res.send(result);
53+
});
54+
}
55+
}

0 commit comments

Comments
 (0)