support subrouters, and engine registrations with file extensions

Author: erik-krogh

javascript/ql/src/semmle/javascript/security/dataflow/TemplateObjectInjectionCustomizations.qll

@@ -53,7 +53,8 @@ module TemplateObjectInjection {
       |
         setup.getARouteHandler() = getRouteHandler() and
         setup.getRouter() = router and
-        usesVulnerableTemplateEngine(router)
+        top.getASubRouter*() = router and
+        usesVulnerableTemplateEngine(top)
       )
     }
   }
@@ -98,7 +99,13 @@ module TemplateObjectInjection {
       viewEngineCall.getMethodName() = "set" and
       viewEngineCall.getArgument(0).getStringValue() = "view engine" and
       // The name set by the `app.engine("name")` call matches `app.set("view engine", "name")`.
-      viewEngineCall.getArgument(1).getStringValue() = registerCall.getArgument(0).getStringValue()
+      (
+        viewEngineCall.getArgument(1).getStringValue() =
+          registerCall.getArgument(0).getStringValue()
+        or
+        "." + viewEngineCall.getArgument(1).getStringValue() =
+          registerCall.getArgument(0).getStringValue()
+      )
     |
       // Different ways of initializing vulnerable template engines.
       engine = DataFlow::moduleImport(getAVulnerableTemplateEngine())

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

@@ -17,6 +17,18 @@ nodes
 | tst2.js:34:25:34:46 | req.bod ... rameter |
 | tst2.js:35:28:35:40 | bodyParameter |
 | tst2.js:35:28:35:40 | bodyParameter |
+| tst2.js:42:9:42:46 | bodyParameter |
+| tst2.js:42:25:42:32 | req.body |
+| tst2.js:42:25:42:32 | req.body |
+| tst2.js:42:25:42:46 | req.bod ... rameter |
+| tst2.js:43:28:43:40 | bodyParameter |
+| tst2.js:43:28:43:40 | bodyParameter |
+| tst2.js:51:9:51:46 | bodyParameter |
+| tst2.js:51:25:51:32 | req.body |
+| tst2.js:51:25:51:32 | req.body |
+| tst2.js:51:25:51:46 | req.bod ... rameter |
+| tst2.js:52:28:52:40 | bodyParameter |
+| tst2.js:52:28:52:40 | bodyParameter |
 | tst.js:5:9:5:46 | bodyParameter |
 | tst.js:5:25:5:32 | req.body |
 | tst.js:5:25:5:32 | req.body |
@@ -58,6 +70,16 @@ edges
 | tst2.js:34:25:34:32 | req.body | tst2.js:34:25:34:46 | req.bod ... rameter |
 | tst2.js:34:25:34:32 | req.body | tst2.js:34:25:34:46 | req.bod ... rameter |
 | tst2.js:34:25:34:46 | req.bod ... rameter | tst2.js:34:9:34:46 | bodyParameter |
+| tst2.js:42:9:42:46 | bodyParameter | tst2.js:43:28:43:40 | bodyParameter |
+| tst2.js:42:9:42:46 | bodyParameter | tst2.js:43:28:43:40 | bodyParameter |
+| tst2.js:42:25:42:32 | req.body | tst2.js:42:25:42:46 | req.bod ... rameter |
+| tst2.js:42:25:42:32 | req.body | tst2.js:42:25:42:46 | req.bod ... rameter |
+| tst2.js:42:25:42:46 | req.bod ... rameter | tst2.js:42:9:42:46 | bodyParameter |
+| tst2.js:51:9:51:46 | bodyParameter | tst2.js:52:28:52:40 | bodyParameter |
+| tst2.js:51:9:51:46 | bodyParameter | tst2.js:52:28:52:40 | bodyParameter |
+| tst2.js:51:25:51:32 | req.body | tst2.js:51:25:51:46 | req.bod ... rameter |
+| tst2.js:51:25:51:32 | req.body | tst2.js:51:25:51:46 | req.bod ... rameter |
+| tst2.js:51:25:51:46 | req.bod ... rameter | tst2.js:51:9:51:46 | bodyParameter |
 | tst.js:5:9:5:46 | bodyParameter | tst.js:8:28:8:40 | bodyParameter |
 | tst.js:5:9:5:46 | bodyParameter | tst.js:8:28:8:40 | bodyParameter |
 | tst.js:5:25:5:32 | req.body | tst.js:5:25:5:46 | req.bod ... rameter |
@@ -85,6 +107,8 @@ edges
 | 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 |
 | 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 |
 | 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 |
+| tst2.js:43:28:43:40 | bodyParameter | tst2.js:42:25:42:32 | req.body | tst2.js:43:28:43:40 | bodyParameter | Template object injection due to $@. | tst2.js:42:25:42:32 | req.body | user-provided value |
+| tst2.js:52:28:52:40 | bodyParameter | tst2.js:51:25:51:32 | req.body | tst2.js:52:28:52:40 | bodyParameter | Template object injection due to $@. | tst2.js:51:25:51:32 | req.body | user-provided value |
 | tst.js:8:28:8:40 | bodyParameter | tst.js:5:25:5:32 | req.body | tst.js:8:28:8:40 | bodyParameter | Template object injection due to $@. | tst.js:5:25:5:32 | req.body | user-provided value |
 | tst.js:9:28:9:41 | queryParameter | tst.js:6:26:6:49 | req.que ... rameter | tst.js:9:28:9:41 | queryParameter | Template object injection due to $@. | tst.js:6:26:6:49 | req.que ... rameter | user-provided value |
 | tst.js:22:28:22:30 | obj | tst.js:6:26:6:49 | req.que ... rameter | tst.js:22:28:22:30 | obj | Template object injection due to $@. | tst.js:6:26:6:49 | req.que ... rameter | user-provided value |

javascript/ql/test/query-tests/Security/CWE-073/tst2.js

@@ -34,3 +34,21 @@ app5.post('/path', function(req, res) {
     var bodyParameter = req.body.bodyParameter;
     res.render('template', bodyParameter); // NOT OK
 }); 
+
+var app6 = require('express')();
+app6.register(".html", require("consolidate").whiskers);
+app6.set('view engine', 'html');
+app6.post('/path', function(req, res) {
+    var bodyParameter = req.body.bodyParameter;
+    res.render('template', bodyParameter); // NOT OK
+}); 
+
+const express = require('express');
+var router = express.Router();
+var app7 = express();
+app7.set('view engine', 'ejs');
+router.post('/path', function(req, res) {
+    var bodyParameter = req.body.bodyParameter;
+    res.render('template', bodyParameter); // NOT OK
+});
+app7.use("/router", router);