precise tracking of handlebars arguments

This commit is contained in:
Stephan Brandauer
2022-03-28 17:26:43 +02:00
parent a28e9c5b6e
commit 9c3fcb6268
3 changed files with 185 additions and 85 deletions

View File

@@ -29,23 +29,66 @@ module Handlebars {
}
/** Provides logic for taint steps for the handlebars library. */
module TaintStep {
private module HandlebarsTaintSteps {
/**
* Gets a `SourceNode` tracked from a compilation of a Handlebars template.
* Gets a reference to a compiled Handlebars template.
*/
private DataFlow::SourceNode compiledHandlebarsTemplate(DataFlow::Node originalCall) {
result = compiledHandlebarsTemplate(DataFlow::TypeTracker::end(), originalCall)
private DataFlow::SourceNode compiledTemplate(DataFlow::CallNode compileCall) {
result = compiledTemplate(DataFlow::TypeTracker::end(), compileCall)
}
private DataFlow::SourceNode compiledHandlebarsTemplate(
DataFlow::TypeTracker t, DataFlow::Node originalCall
private DataFlow::SourceNode compiledTemplate(
DataFlow::TypeTracker t, DataFlow::CallNode compileCall
) {
t.start() and
result = any(Handlebars::Handlebars hb).getAMethodCall(["compile", "template"]) and
result = originalCall
result = compileCall
or
exists(DataFlow::TypeTracker t2 |
result = compiledHandlebarsTemplate(t2, originalCall).track(t2, t)
exists(DataFlow::TypeTracker t2 | result = compiledTemplate(t2, compileCall).track(t2, t))
}
private predicate isHelperParam(
string helperName, DataFlow::FunctionNode helperFunction, DataFlow::ParameterNode param,
int paramIndex
) {
exists(DataFlow::CallNode registerHelperCall |
registerHelperCall = any(Handlebars::Handlebars hb).getAMethodCall("registerHelper") and
registerHelperCall.getArgument(0).mayHaveStringValue(helperName) and
helperFunction = registerHelperCall.getArgument(1).getAFunctionValue() and
param = helperFunction.getParameter(paramIndex)
)
}
/** Holds if `call` is a block wrapped inside curly braces inside the template `templateText`. */
bindingset[templateText]
predicate templateHelperParamBlock(string templateText, string call) {
call = templateText.regexpFind("\\{\\{[^}]+\\}\\}", _, _).regexpReplaceAll("[{}]", "").trim() and
call.regexpMatch(".*\\s.*")
}
/**
* Holds for calls to helpers from handlebars templates.
*
* ```javascript
* hb.compile("contents of file {{path}} are: {{catFile path}} {{echo p1 p2}}");
* ```
*
* In the example, the predicate will hold for:
*
* * helperName="catFile", argIdx=1, arg="path"
* * helperName="echo", argIdx=1, arg="p1"
* * helperName="echo", argIdx=2, arg="p2"
*
* The initial `{{path}}` will not be considered, as it has no arguments.
*/
bindingset[templateText]
private predicate isTemplateHelperCallArg(
string templateText, string helperName, int argIdx, string argVal
) {
exists(string call | templateHelperParamBlock(templateText, call) |
helperName = call.regexpFind("[^\\s]+", 0, _) and
argIdx >= 0 and
argVal = call.regexpFind("[^\\s]+", argIdx + 1, _)
)
}
@@ -71,29 +114,26 @@ module TaintStep {
* ```
*/
private predicate isHandlebarsArgStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(string helperName |
exists(DataFlow::CallNode templatingCall, DataFlow::CallNode compileCall |
templatingCall = compiledHandlebarsTemplate(compileCall).getACall() and
pred = templatingCall.getAnArgument().getALocalSource().getAPropertyWrite().getRhs() and
(
compileCall
.getArgument(0)
.mayHaveStringValue(any(string templateText |
// an approximation: if the template contains the
// helper name, it's probably a helper call
templateText.matches("%" + helperName + "%")
))
or
// When we don't have a string value, we can't be sure
// and will assume a step.
not exists(string s | compileCall.getArgument(0).mayHaveStringValue(s))
exists(
string helperName, DataFlow::CallNode templatingCall, DataFlow::CallNode compileCall,
DataFlow::FunctionNode helperFunction
|
templatingCall = compiledTemplate(compileCall).getACall() and
(
exists(string templateText, string paramName, int argIdx |
compileCall.getArgument(0).mayHaveStringValue(templateText)
|
pred =
templatingCall.getAnArgument().getALocalSource().getAPropertyWrite(paramName).getRhs() and
isTemplateHelperCallArg(templateText, helperName, argIdx, paramName) and
isHelperParam(helperName, helperFunction, succ, argIdx)
)
) and
exists(DataFlow::CallNode registerHelperCall, DataFlow::FunctionNode helperFunc |
registerHelperCall = any(Handlebars::Handlebars hb).getAMethodCall("registerHelper") and
registerHelperCall.getArgument(0).mayHaveStringValue(helperName) and
helperFunc = registerHelperCall.getArgument(1).getAFunctionValue() and
helperFunc.getAParameter() = succ
or
// When we don't have a string value, we can't be sure
// and will assume a step.
not exists(string s | compileCall.getArgument(0).mayHaveStringValue(s)) and
pred = templatingCall.getAnArgument().getALocalSource().getAPropertyWrite().getRhs() and
isHelperParam(helperName, helperFunction, succ, _)
)
)
}
@@ -105,8 +145,8 @@ module TaintStep {
class HandlebarsStep extends DataFlow::SharedFlowStep {
DataFlow::CallNode templatingCall;
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
isHandlebarsArgStep(node1, node2)
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
isHandlebarsArgStep(pred, succ)
}
}
}

View File

@@ -1541,25 +1541,39 @@ nodes
| express.js:8:20:8:32 | req.query.bar |
| express.js:8:20:8:32 | req.query.bar |
| express.js:8:20:8:32 | req.query.bar |
| handlebars.js:19:46:19:60 | req.params.path |
| handlebars.js:19:46:19:60 | req.params.path |
| handlebars.js:19:46:19:60 | req.params.path |
| handlebars.js:19:46:19:60 | req.params.path |
| handlebars.js:19:46:19:60 | req.params.path |
| handlebars.js:22:18:22:25 | filePath |
| handlebars.js:22:18:22:25 | filePath |
| handlebars.js:22:18:22:25 | filePath |
| handlebars.js:22:18:22:25 | filePath |
| handlebars.js:23:28:23:35 | filePath |
| handlebars.js:23:28:23:35 | filePath |
| handlebars.js:23:28:23:35 | filePath |
| handlebars.js:23:28:23:35 | filePath |
| handlebars.js:23:28:23:35 | filePath |
| handlebars.js:31:43:31:57 | req.params.name |
| handlebars.js:31:43:31:57 | req.params.name |
| handlebars.js:31:43:31:57 | req.params.name |
| handlebars.js:31:43:31:57 | req.params.name |
| handlebars.js:31:43:31:57 | req.params.name |
| handlebars.js:10:51:10:58 | filePath |
| handlebars.js:10:51:10:58 | filePath |
| handlebars.js:10:51:10:58 | filePath |
| handlebars.js:10:51:10:58 | filePath |
| handlebars.js:11:32:11:39 | filePath |
| handlebars.js:11:32:11:39 | filePath |
| handlebars.js:11:32:11:39 | filePath |
| handlebars.js:11:32:11:39 | filePath |
| handlebars.js:11:32:11:39 | filePath |
| handlebars.js:13:73:13:80 | filePath |
| handlebars.js:13:73:13:80 | filePath |
| handlebars.js:13:73:13:80 | filePath |
| handlebars.js:13:73:13:80 | filePath |
| handlebars.js:15:25:15:32 | filePath |
| handlebars.js:15:25:15:32 | filePath |
| handlebars.js:15:25:15:32 | filePath |
| handlebars.js:15:25:15:32 | filePath |
| handlebars.js:15:25:15:32 | filePath |
| handlebars.js:29:46:29:60 | req.params.path |
| handlebars.js:29:46:29:60 | req.params.path |
| handlebars.js:29:46:29:60 | req.params.path |
| handlebars.js:29:46:29:60 | req.params.path |
| handlebars.js:29:46:29:60 | req.params.path |
| handlebars.js:37:43:37:57 | req.params.name |
| handlebars.js:37:43:37:57 | req.params.name |
| handlebars.js:37:43:37:57 | req.params.name |
| handlebars.js:37:43:37:57 | req.params.name |
| handlebars.js:37:43:37:57 | req.params.name |
| handlebars.js:43:15:43:29 | req.params.path |
| handlebars.js:43:15:43:29 | req.params.path |
| handlebars.js:43:15:43:29 | req.params.path |
| handlebars.js:43:15:43:29 | req.params.path |
| handlebars.js:43:15:43:29 | req.params.path |
| normalizedPaths.js:11:7:11:27 | path |
| normalizedPaths.js:11:7:11:27 | path |
| normalizedPaths.js:11:7:11:27 | path |
@@ -6433,30 +6447,54 @@ edges
| TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) |
| TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:211:14:211:37 | url.par ... , true) |
| express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:19:46:19:60 | req.params.path | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:22:18:22:25 | filePath | handlebars.js:23:28:23:35 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:31:43:31:57 | req.params.name | handlebars.js:22:18:22:25 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:10:51:10:58 | filePath | handlebars.js:11:32:11:39 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:13:73:13:80 | filePath | handlebars.js:15:25:15:32 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:29:46:29:60 | req.params.path | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:10:51:10:58 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:37:43:37:57 | req.params.name | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| handlebars.js:43:15:43:29 | req.params.path | handlebars.js:13:73:13:80 | filePath |
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
| normalizedPaths.js:11:7:11:27 | path | normalizedPaths.js:13:19:13:22 | path |
@@ -9887,8 +9925,10 @@ edges
| TaintedPath.js:213:45:213:48 | path | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:213:45:213:48 | path | This path depends on $@. | TaintedPath.js:211:24:211:30 | req.url | a user-provided value |
| TaintedPath.js:214:35:214:38 | path | TaintedPath.js:211:24:211:30 | req.url | TaintedPath.js:214:35:214:38 | path | This path depends on $@. | TaintedPath.js:211:24:211:30 | req.url | a user-provided value |
| express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | express.js:8:20:8:32 | req.query.bar | This path depends on $@. | express.js:8:20:8:32 | req.query.bar | a user-provided value |
| handlebars.js:23:28:23:35 | filePath | handlebars.js:19:46:19:60 | req.params.path | handlebars.js:23:28:23:35 | filePath | This path depends on $@. | handlebars.js:19:46:19:60 | req.params.path | a user-provided value |
| handlebars.js:23:28:23:35 | filePath | handlebars.js:31:43:31:57 | req.params.name | handlebars.js:23:28:23:35 | filePath | This path depends on $@. | handlebars.js:31:43:31:57 | req.params.name | a user-provided value |
| handlebars.js:11:32:11:39 | filePath | handlebars.js:29:46:29:60 | req.params.path | handlebars.js:11:32:11:39 | filePath | This path depends on $@. | handlebars.js:29:46:29:60 | req.params.path | a user-provided value |
| handlebars.js:11:32:11:39 | filePath | handlebars.js:37:43:37:57 | req.params.name | handlebars.js:11:32:11:39 | filePath | This path depends on $@. | handlebars.js:37:43:37:57 | req.params.name | a user-provided value |
| handlebars.js:15:25:15:32 | filePath | handlebars.js:37:43:37:57 | req.params.name | handlebars.js:15:25:15:32 | filePath | This path depends on $@. | handlebars.js:37:43:37:57 | req.params.name | a user-provided value |
| handlebars.js:15:25:15:32 | filePath | handlebars.js:43:15:43:29 | req.params.path | handlebars.js:15:25:15:32 | filePath | This path depends on $@. | handlebars.js:43:15:43:29 | req.params.path | a user-provided value |
| normalizedPaths.js:13:19:13:22 | path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:13:19:13:22 | path | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |
| normalizedPaths.js:14:19:14:29 | './' + path | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:14:19:14:29 | './' + path | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |
| normalizedPaths.js:15:19:15:38 | path + '/index.html' | normalizedPaths.js:11:14:11:27 | req.query.path | normalizedPaths.js:15:19:15:38 | path + '/index.html' | This path depends on $@. | normalizedPaths.js:11:14:11:27 | req.query.path | a user-provided value |

View File

@@ -7,10 +7,20 @@ const app = express();
const data = {};
function init() {
hb.registerHelper("catFile", catFile);
hb.registerHelper("catFile", function catFile(filePath) {
return fs.readFileSync(filePath); // SINK (reads file)
});
hb.registerHelper("prependToLines", function prependToLines(prefix, filePath) {
return fs
.readFileSync(filePath)
.split("\n")
.map((line) => prefix + line)
.join("\n");
});
data.compiledFileAccess = hb.compile("contents of file {{path}} are: {{catFile path}}")
data.compiledBenign = hb.compile("hello, {{name}}");
data.compiledUnknown = hb.compile(fs.readFileSync("greeting.template"));
data.compiledMixed = hb.compile("helpers may have several args, like here: {{prependToLines prefix path}}");
}
init();
@@ -19,14 +29,24 @@ app.get('/some/path1', function (req, res) {
res.send(data.compiledFileAccess({ path: req.params.path })); // NOT ALLOWED (template uses vulnerable catFile)
});
function catFile(filePath) {
return fs.readFileSync(filePath); // SINK (reads file)
}
app.get('/some/path2', function (req, res) {
res.send(data.compiledBenign({ name: req.params.name })); // ALLOWED (this template does not use catFile)
});
app.get('/some/path3', function (req, res) {
res.send(data.compiledUnknown({ name: req.params.name })); // NOT ALLOWED (could be using vulnerable catFile)
app.get('/some/path3', function (req, res) {
res.send(data.compiledUnknown({ name: req.params.name })); // NOT ALLOWED (could be using a vulnerable helper)
});
app.get('/some/path4', function (req, res) {
res.send(data.compiledMixed({
prefix: ">>> ",
path: req.params.path // NOT ALLOWED (template uses vulnerable helper)
}));
});
app.get('/some/path5', function (req, res) {
res.send(data.compiledMixed({
prefix: req.params.prefix, // ALLOWED (this parameter is safe)
path: "data/path-5.txt"
}));
});