Skip to content

Commit da8e67b

Browse files
committed
JS: Use routing trees to detect deeply tainted req.body
1 parent 7492293 commit da8e67b

File tree

3 files changed

+8
-5
lines changed

3 files changed

+8
-5
lines changed

javascript/ql/lib/semmle/javascript/frameworks/Express.qll

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -613,10 +613,9 @@ module Express {
613613

614614
override predicate isUserControlledObject() {
615615
kind = "body" and
616-
exists(ExpressLibraries::BodyParser bodyParser, RouteHandlerExpr expr |
617-
expr.getBody() = request.getRouteHandler() and
618-
bodyParser.producesUserControlledObjects() and
619-
bodyParser.flowsToExpr(expr.getAMatchingAncestor())
616+
exists(ExpressLibraries::BodyParser bodyParser |
617+
Routing::getNode(request.getRouteHandler()).isGuardedBy(bodyParser) and
618+
bodyParser.producesUserControlledObjects()
620619
)
621620
or
622621
// If we can't find the middlewares for the route handler,
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
| query-tests/Security/CWE-073/routes.js:2 | expected an alert, but found none | NOT OK | |

javascript/ql/test/query-tests/Security/CWE-073/TemplateObjectInjection.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
nodes
2+
| routes.js:2:23:2:30 | req.body |
3+
| routes.js:2:23:2:30 | req.body |
4+
| routes.js:2:23:2:30 | req.body |
25
| tst2.js:6:9:6:46 | bodyParameter |
36
| tst2.js:6:25:6:32 | req.body |
47
| tst2.js:6:25:6:32 | req.body |
@@ -55,6 +58,7 @@ nodes
5558
| tst.js:29:28:29:42 | JSON.parse(str) |
5659
| tst.js:29:39:29:41 | str |
5760
edges
61+
| routes.js:2:23:2:30 | req.body | routes.js:2:23:2:30 | req.body |
5862
| tst2.js:6:9:6:46 | bodyParameter | tst2.js:7:28:7:40 | bodyParameter |
5963
| tst2.js:6:9:6:46 | bodyParameter | tst2.js:7:28:7:40 | bodyParameter |
6064
| tst2.js:6:25:6:32 | req.body | tst2.js:6:25:6:46 | req.bod ... rameter |
@@ -104,6 +108,7 @@ edges
104108
| tst.js:29:39:29:41 | str | tst.js:29:28:29:42 | JSON.parse(str) |
105109
| tst.js:29:39:29:41 | str | tst.js:29:28:29:42 | JSON.parse(str) |
106110
#select
111+
| routes.js:2:23:2:30 | req.body | routes.js:2:23:2:30 | req.body | routes.js:2:23:2:30 | req.body | Template object injection due to $@. | routes.js:2:23:2:30 | req.body | user-provided value |
107112
| tst2.js:7:28:7:40 | bodyParameter | tst2.js:6:25:6:32 | req.body | tst2.js:7:28:7:40 | bodyParameter | Template object injection due to $@. | tst2.js:6:25:6:32 | req.body | user-provided value |
108113
| tst2.js:27:28:27:40 | bodyParameter | tst2.js:26:25:26:32 | req.body | tst2.js:27:28:27:40 | bodyParameter | Template object injection due to $@. | tst2.js:26:25:26:32 | req.body | user-provided value |
109114
| tst2.js:35:28:35:40 | bodyParameter | tst2.js:34:25:34:32 | req.body | tst2.js:35:28:35:40 | bodyParameter | Template object injection due to $@. | tst2.js:34:25:34:32 | req.body | user-provided value |

0 commit comments

Comments
 (0)