Skip to content

Commit

Permalink
Sanitize option names.
Browse files Browse the repository at this point in the history
This prevents injection of arbitrary code if the server is already
vulnerable to prototype poisoning. This resolves #451.

I deliberately opted to not support complex Unicode identifiers even
though they're valid JS identifiers. They're complex to validate and
users probably shouldn't even try to be that creative.
  • Loading branch information
nicdumz committed May 30, 2021
1 parent c120527 commit 15ee698
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 0 deletions.
10 changes: 10 additions & 0 deletions lib/ejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ var _OPTS_PASSABLE_WITH_DATA = ['delimiter', 'scope', 'context', 'debug', 'compi
// so we make an exception for `renderFile`
var _OPTS_PASSABLE_WITH_DATA_EXPRESS = _OPTS_PASSABLE_WITH_DATA.concat('cache');
var _BOM = /^\uFEFF/;
var _JS_IDENTIFIER = /^[a-zA-Z_$][0-9a-zA-Z_$]*$/;

/**
* EJS template function cache. This can be a LRU object from lru-cache NPM
Expand Down Expand Up @@ -587,12 +588,21 @@ Template.prototype = {
' var __output = "";\n' +
' function __append(s) { if (s !== undefined && s !== null) __output += s }\n';
if (opts.outputFunctionName) {
if (!_JS_IDENTIFIER.test(opts.outputFunctionName)) {
throw new Error('outputFunctionName is not a valid JS identifier.');
}
prepended += ' var ' + opts.outputFunctionName + ' = __append;' + '\n';
}
if (opts.localsName && !_JS_IDENTIFIER.test(opts.localsName)) {
throw new Error('localsName is not a valid JS identifier.');
}
if (opts.destructuredLocals && opts.destructuredLocals.length) {
var destructuring = ' var __locals = (' + opts.localsName + ' || {}),\n';
for (var i = 0; i < opts.destructuredLocals.length; i++) {
var name = opts.destructuredLocals[i];
if (!_JS_IDENTIFIER.test(name)) {
throw new Error(`destructuredLocals[${i}] is not a valid JS identifier.`);
}
if (i > 0) {
destructuring += ',\n ';
}
Expand Down
26 changes: 26 additions & 0 deletions test/ejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1178,3 +1178,29 @@ suite('meta information', function () {
assert.strictEqual(ejs.name, 'ejs');
});
});

suite('identifier validation', function () {
test('invalid outputFunctionName', function() {
assert.throws(function() {
ejs.compile('<p>yay</p>', {outputFunctionName: 'x;console.log(1);x'});
}, /outputFunctionName is not a valid JS identifier/)
});

test('invalid localsName', function() {
var locals = Object.create(null);
assert.throws(function() {
ejs.compile('<p>yay</p>', {
localsName: 'function(){console.log(1);return locals;}()'});
}, /localsName is not a valid JS identifier/)
});

test('invalid destructuredLocals', function() {
var locals = {};
assert.throws(function() {
ejs.compile('<p>yay</p>', {
destructuredLocals: [
'console.log(1); //'
]});
}, /destructuredLocals\[0\] is not a valid JS identifier/)
});
});

0 comments on commit 15ee698

Please sign in to comment.