mirror of
https://github.com/github/codeql.git
synced 2026-05-01 03:35:13 +02:00
add engine filter to js/template-object-injection
This commit is contained in:
Erik Krogh Kristensen
@@ -42,7 +42,83 @@ module TemplateObjectInjection {
|
||||
override DataFlow::FlowLabel getAFlowLabel() { result.isTaint() }
|
||||
}
|
||||
|
||||
private class TemplateSink extends Sink {
|
||||
TemplateSink() { this.asExpr() instanceof Express::TemplateObjectInput }
|
||||
/**
|
||||
* An argument given to the `render` method on an Express response object,
|
||||
* where the view engine used by the Express instance is vulnerable to template object injection.
|
||||
*/
|
||||
private class TemplateSink extends Sink, Express::TemplateObjectInput {
|
||||
TemplateSink() {
|
||||
exists(
|
||||
Express::RouteSetup setup, Express::RouterDefinition router, Express::RouterDefinition top
|
||||
|
|
||||
setup.getARouteHandler() = getRouteHandler() and
|
||||
setup.getRouter() = router and
|
||||
usesVulnerableTemplateEngine(router)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the package name for a templating library that is vulnerable to template object injection.
|
||||
*/
|
||||
private string getAVulnerableTemplateEngine() {
|
||||
result =
|
||||
[
|
||||
"eta", "squirrelly", "haml-coffee", "express-hbs", "ejs", "hbs", "whiskers",
|
||||
"express-handlebars"
|
||||
]
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if the "view engine" of `router` is set to a vulnerable templating engine.
|
||||
*/
|
||||
predicate usesVulnerableTemplateEngine(Express::RouterDefinition router) {
|
||||
// option 1: `app.set("view engine", "theEngine")`.
|
||||
// Express will load the engine automatically.
|
||||
exists(MethodCallExpr call |
|
||||
router.flowsTo(call.getReceiver()) and
|
||||
call.getMethodName() = "set" and
|
||||
call.getArgument(0).getStringValue() = "view engine" and
|
||||
call.getArgument(1).getStringValue() = getAVulnerableTemplateEngine()
|
||||
)
|
||||
or
|
||||
// option 2: setup an engine.
|
||||
// ```app.engine("name", engine); app.set("view engine", "name");```
|
||||
// ^^^ `registerCall` ^^^ ^^^ `viewEngineCall` ^^^
|
||||
exists(
|
||||
DataFlow::MethodCallNode registerCall, DataFlow::SourceNode engine,
|
||||
DataFlow::MethodCallNode viewEngineCall
|
||||
|
|
||||
// `app.engine("name", engine)
|
||||
router.flowsTo(registerCall.getReceiver().asExpr()) and
|
||||
registerCall.getMethodName() = ["engine", "register"] and
|
||||
engine = registerCall.getArgument(1).getALocalSource() and
|
||||
// app.set("view engine", "name")
|
||||
router.flowsTo(viewEngineCall.getReceiver().asExpr()) and
|
||||
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()
|
||||
|
|
||||
// Different ways of initializing vulnerable template engines.
|
||||
engine = DataFlow::moduleImport(getAVulnerableTemplateEngine())
|
||||
or
|
||||
engine = DataFlow::moduleImport(getAVulnerableTemplateEngine()).getAPropertyRead("__express")
|
||||
or
|
||||
engine = DataFlow::moduleImport("express-handlebars").getACall()
|
||||
or
|
||||
engine = DataFlow::moduleImport("express-handlebars").getAPropertyRead("engine")
|
||||
or
|
||||
exists(DataFlow::SourceNode hbs |
|
||||
hbs = DataFlow::moduleImport("express-hbs")
|
||||
or
|
||||
hbs = DataFlow::moduleImport("express-hbs").getAMemberCall("create")
|
||||
|
|
||||
engine = hbs.getAMemberCall(["express3", "express4"])
|
||||
)
|
||||
or
|
||||
engine =
|
||||
DataFlow::moduleImport("consolidate").getAPropertyRead(getAVulnerableTemplateEngine())
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,22 @@
|
||||
nodes
|
||||
| tst2.js:6:9:6:46 | bodyParameter |
|
||||
| tst2.js:6:25:6:32 | req.body |
|
||||
| tst2.js:6:25:6:32 | req.body |
|
||||
| tst2.js:6:25:6:46 | req.bod ... rameter |
|
||||
| tst2.js:7:28:7:40 | bodyParameter |
|
||||
| tst2.js:7:28:7:40 | bodyParameter |
|
||||
| tst2.js:26:9:26:46 | bodyParameter |
|
||||
| tst2.js:26:25:26:32 | req.body |
|
||||
| tst2.js:26:25:26:32 | req.body |
|
||||
| tst2.js:26:25:26:46 | req.bod ... rameter |
|
||||
| tst2.js:27:28:27:40 | bodyParameter |
|
||||
| tst2.js:27:28:27:40 | bodyParameter |
|
||||
| tst2.js:34:9:34:46 | bodyParameter |
|
||||
| tst2.js:34:25:34:32 | req.body |
|
||||
| tst2.js:34:25:34:32 | req.body |
|
||||
| tst2.js:34:25:34:46 | req.bod ... rameter |
|
||||
| tst2.js:35:28:35:40 | bodyParameter |
|
||||
| tst2.js:35:28:35: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 |
|
||||
@@ -25,6 +43,21 @@ nodes
|
||||
| tst.js:27:28:27:42 | JSON.parse(str) |
|
||||
| tst.js:27:39:27:41 | str |
|
||||
edges
|
||||
| tst2.js:6:9:6:46 | bodyParameter | tst2.js:7:28:7:40 | bodyParameter |
|
||||
| tst2.js:6:9:6:46 | bodyParameter | tst2.js:7:28:7:40 | bodyParameter |
|
||||
| tst2.js:6:25:6:32 | req.body | tst2.js:6:25:6:46 | req.bod ... rameter |
|
||||
| tst2.js:6:25:6:32 | req.body | tst2.js:6:25:6:46 | req.bod ... rameter |
|
||||
| tst2.js:6:25:6:46 | req.bod ... rameter | tst2.js:6:9:6:46 | bodyParameter |
|
||||
| tst2.js:26:9:26:46 | bodyParameter | tst2.js:27:28:27:40 | bodyParameter |
|
||||
| tst2.js:26:9:26:46 | bodyParameter | tst2.js:27:28:27:40 | bodyParameter |
|
||||
| tst2.js:26:25:26:32 | req.body | tst2.js:26:25:26:46 | req.bod ... rameter |
|
||||
| tst2.js:26:25:26:32 | req.body | tst2.js:26:25:26:46 | req.bod ... rameter |
|
||||
| tst2.js:26:25:26:46 | req.bod ... rameter | tst2.js:26:9:26:46 | bodyParameter |
|
||||
| tst2.js:34:9:34:46 | bodyParameter | tst2.js:35:28:35:40 | bodyParameter |
|
||||
| tst2.js:34:9:34:46 | bodyParameter | tst2.js:35:28:35:40 | bodyParameter |
|
||||
| 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 |
|
||||
| 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 |
|
||||
@@ -49,6 +82,9 @@ edges
|
||||
| tst.js:27:39:27:41 | str | tst.js:27:28:27:42 | JSON.parse(str) |
|
||||
| tst.js:27:39:27:41 | str | tst.js:27:28:27:42 | JSON.parse(str) |
|
||||
#select
|
||||
| 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 |
|
||||
| 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 |
|
||||
|
||||
36
javascript/ql/test/query-tests/Security/CWE-073/tst2.js
Normal file
36
javascript/ql/test/query-tests/Security/CWE-073/tst2.js
Normal file
@@ -0,0 +1,36 @@
|
||||
const handlebars = require("express-handlebars");
|
||||
var app = require('express')();
|
||||
app.engine( '.hbs', handlebars({ defaultLayout: 'main', extname: '.hbs' }) );
|
||||
app.set('view engine', '.hbs')
|
||||
app.post('/path', function(req, res) {
|
||||
var bodyParameter = req.body.bodyParameter;
|
||||
res.render('template', bodyParameter); // NOT OK
|
||||
});
|
||||
|
||||
var app2 = require('express')();
|
||||
app2.post('/path', function(req, res) {
|
||||
var bodyParameter = req.body.bodyParameter;
|
||||
res.render('template', bodyParameter); // OK
|
||||
});
|
||||
|
||||
var app3 = require('express')();
|
||||
app3.set('view engine', 'pug');
|
||||
app3.post('/path', function(req, res) {
|
||||
var bodyParameter = req.body.bodyParameter;
|
||||
res.render('template', bodyParameter); // OK
|
||||
});
|
||||
|
||||
var app4 = require('express')();
|
||||
app4.set('view engine', 'ejs');
|
||||
app4.post('/path', function(req, res) {
|
||||
var bodyParameter = req.body.bodyParameter;
|
||||
res.render('template', bodyParameter); // NOT OK
|
||||
});
|
||||
|
||||
var app5 = require('express')();
|
||||
app5.engine("foobar", require("consolidate").whiskers);
|
||||
app5.set('view engine', 'foobar');
|
||||
app5.post('/path', function(req, res) {
|
||||
var bodyParameter = req.body.bodyParameter;
|
||||
res.render('template', bodyParameter); // NOT OK
|
||||
});
|
||||
Reference in New Issue
Block a user