Skip to content

Commit 408ac27

Browse files
erik-krogherik-krogh
authored
Merge pull request #5066 from CaptainFreak/express-hbs-lfr
JS: add query for Express-HBS LFR
2 parents 5188ad1 + 503b339 commit 408ac27

File tree

6 files changed

+153
-0
lines changed

6 files changed

+153
-0
lines changed

‎javascript/ql/src/experimental/Security/CWE-073/TemplateObjectInjection.ql‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/**
2+
* @name Template Object Injection
3+
* @description Instantiating a template using a user-controlled object is vulnerable to local file read and potential remote code execution.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @precision high
7+
* @id js/template-object-injection
8+
* @tags security
9+
* external/cwe/cwe-073
10+
* external/cwe/cwe-094
11+
*/
12+
13+
import javascript
14+
import DataFlow::PathGraph
15+
import semmle.javascript.security.TaintedObject
16+
17+
class TemplateObjInjectionConfig extends TaintTracking::Configuration {
18+
TemplateObjInjectionConfig() { this = "TemplateObjInjectionConfig" }
19+
20+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
21+
22+
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
23+
TaintedObject::isSource(source, label)
24+
}
25+
26+
override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
27+
label = TaintedObject::label() and
28+
exists(MethodCallExpr mc |
29+
Express::isResponse(mc.getReceiver()) and
30+
mc.getMethodName() = "render" and
31+
sink.asExpr() = mc.getArgument(1)
32+
)
33+
}
34+
35+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
36+
guard instanceof TaintedObject::SanitizerGuard
37+
}
38+
39+
override predicate isAdditionalFlowStep(
40+
DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
41+
) {
42+
TaintedObject::step(src, trg, inlbl, outlbl)
43+
}
44+
}
45+
46+
from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
47+
where cfg.hasFlowPath(source, sink)
48+
select sink.getNode(), source, sink, "Template object injection due to $@.", source.getNode(),
49+
"user-provided value"

‎javascript/ql/src/experimental/Security/CWE-073/documentation-examples/TemplateObjectInjection.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var app = require('express')();
2+
app.set('view engine', 'hbs');
3+
4+
app.post('/path', function(req, res) {
5+
var bodyParameter = req.body.bodyParameter
6+
var queryParameter = req.query.queryParameter
7+
8+
res.render('template', bodyParameter)
9+
res.render('template', queryParameter)
10+
});

‎javascript/ql/src/experimental/Security/CWE-073/documentation-examples/TemplateObjectInjection_fixed.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var app = require('express')();
2+
app.set('view engine', 'hbs');
3+
4+
app.post('/path', function(req, res) {
5+
var bodyParameter = req.body.bodyParameter
6+
var queryParameter = req.query.queryParameter
7+
8+
res.render('template', {bodyParameter})
9+
res.render('template', {queryParameter})
10+
});

‎javascript/ql/test/experimental/Security/CWE-073/TemplateObjectInjection.expected‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
nodes
2+
| tst.js:5:9:5:46 | bodyParameter |
3+
| tst.js:5:25:5:32 | req.body |
4+
| tst.js:5:25:5:32 | req.body |
5+
| tst.js:5:25:5:46 | req.bod ... rameter |
6+
| tst.js:6:9:6:49 | queryParameter |
7+
| tst.js:6:9:6:49 | queryParameter |
8+
| tst.js:6:26:6:49 | req.que ... rameter |
9+
| tst.js:6:26:6:49 | req.que ... rameter |
10+
| tst.js:6:26:6:49 | req.que ... rameter |
11+
| tst.js:8:28:8:40 | bodyParameter |
12+
| tst.js:8:28:8:40 | bodyParameter |
13+
| tst.js:9:28:9:41 | queryParameter |
14+
| tst.js:9:28:9:41 | queryParameter |
15+
| tst.js:18:19:18:32 | queryParameter |
16+
| tst.js:18:19:18:32 | queryParameter |
17+
| tst.js:21:24:21:26 | obj |
18+
| tst.js:21:24:21:26 | obj |
19+
| tst.js:22:28:22:30 | obj |
20+
| tst.js:22:28:22:30 | obj |
21+
| tst.js:24:11:24:24 | str |
22+
| tst.js:24:17:24:19 | obj |
23+
| tst.js:24:17:24:24 | obj + "" |
24+
| tst.js:27:28:27:42 | JSON.parse(str) |
25+
| tst.js:27:28:27:42 | JSON.parse(str) |
26+
| tst.js:27:39:27:41 | str |
27+
edges
28+
| tst.js:5:9:5:46 | bodyParameter | tst.js:8:28:8:40 | bodyParameter |
29+
| tst.js:5:9:5:46 | bodyParameter | tst.js:8:28:8:40 | bodyParameter |
30+
| tst.js:5:25:5:32 | req.body | tst.js:5:25:5:46 | req.bod ... rameter |
31+
| tst.js:5:25:5:32 | req.body | tst.js:5:25:5:46 | req.bod ... rameter |
32+
| tst.js:5:25:5:46 | req.bod ... rameter | tst.js:5:9:5:46 | bodyParameter |
33+
| tst.js:6:9:6:49 | queryParameter | tst.js:9:28:9:41 | queryParameter |
34+
| tst.js:6:9:6:49 | queryParameter | tst.js:9:28:9:41 | queryParameter |
35+
| tst.js:6:9:6:49 | queryParameter | tst.js:18:19:18:32 | queryParameter |
36+
| tst.js:6:9:6:49 | queryParameter | tst.js:18:19:18:32 | queryParameter |
37+
| tst.js:6:26:6:49 | req.que ... rameter | tst.js:6:9:6:49 | queryParameter |
38+
| tst.js:6:26:6:49 | req.que ... rameter | tst.js:6:9:6:49 | queryParameter |
39+
| tst.js:6:26:6:49 | req.que ... rameter | tst.js:6:9:6:49 | queryParameter |
40+
| tst.js:6:26:6:49 | req.que ... rameter | tst.js:6:9:6:49 | queryParameter |
41+
| tst.js:18:19:18:32 | queryParameter | tst.js:21:24:21:26 | obj |
42+
| tst.js:18:19:18:32 | queryParameter | tst.js:21:24:21:26 | obj |
43+
| tst.js:21:24:21:26 | obj | tst.js:22:28:22:30 | obj |
44+
| tst.js:21:24:21:26 | obj | tst.js:22:28:22:30 | obj |
45+
| tst.js:21:24:21:26 | obj | tst.js:24:17:24:19 | obj |
46+
| tst.js:24:11:24:24 | str | tst.js:27:39:27:41 | str |
47+
| tst.js:24:17:24:19 | obj | tst.js:24:17:24:24 | obj + "" |
48+
| tst.js:24:17:24:24 | obj + "" | tst.js:24:11:24:24 | str |
49+
| tst.js:27:39:27:41 | str | tst.js:27:28:27:42 | JSON.parse(str) |
50+
| tst.js:27:39:27:41 | str | tst.js:27:28:27:42 | JSON.parse(str) |
51+
#select
52+
| 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 |
53+
| 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 |
54+
| 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 |
55+
| tst.js:27:28:27:42 | JSON.parse(str) | tst.js:6:26:6:49 | req.que ... rameter | tst.js:27:28:27:42 | JSON.parse(str) | Template object injection due to $@. | tst.js:6:26:6:49 | req.que ... rameter | user-provided value |

‎javascript/ql/test/experimental/Security/CWE-073/TemplateObjectInjection.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-073/TemplateObjectInjection.ql

‎javascript/ql/test/experimental/Security/CWE-073/tst.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
var app = require('express')();
2+
app.set('view engine', 'hbs');
3+
4+
app.post('/path', function(req, res) {
5+
var bodyParameter = req.body.bodyParameter;
6+
var queryParameter = req.query.queryParameter;
7+
8+
res.render('template', bodyParameter); // NOT OK
9+
res.render('template', queryParameter); // NOT OK
10+
11+
if (typeof bodyParameter === "string") {
12+
res.render('template', bodyParameter); // OK
13+
}
14+
res.render('template', queryParameter + ""); // OK
15+
16+
res.render('template', {profile: bodyParameter}); // OK
17+
18+
indirect(res, queryParameter);
19+
});
20+
21+
function indirect(res, obj) {
22+
res.render("template", obj); // NOT OK
23+
24+
const str = obj + "";
25+
res.render("template", str); // OK
26+
27+
res.render("template", JSON.parse(str)); // NOT OK
28+
}

0 commit comments

Comments
 (0)