Merge pull request #601 from nicdumz/main

Mitigate prototype pollution effects
mde · May 31, 2021 · 61b6616 · 61b6616
2 parents c120527 + be9a9bb
commit 61b6616
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 8 deletions.
diff --git a/lib/ejs.js b/lib/ejs.js
@@ -44,6 +44,7 @@
  * @public
  */
 
+
 var fs = require('fs');
 var path = require('path');
 var utils = require('./utils');
@@ -64,6 +65,18 @@ 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_$]*$/;
+
+var createObj = function() {
+  if (typeof Object.create !== 'function') {
+    return function (o) {
+      function F() {}
+      F.prototype = o;
+      return new F();
+    };
+  }
+  return Object.create;
+}();
 
 /**
  * EJS template function cache. This can be a LRU object from lru-cache NPM
@@ -305,7 +318,7 @@ function fileLoader(filePath){
  */
 
 function includeFile(path, options) {
-  var opts = utils.shallowCopy({}, options);
+  var opts = utils.shallowCopy(createObj(null), options);
   opts.filename = getIncludePath(path, opts);
   if (typeof options.includer === 'function') {
     var includerResult = options.includer(path, opts.filename);
@@ -411,8 +424,8 @@ exports.compile = function compile(template, opts) {
  */
 
 exports.render = function (template, d, o) {
-  var data = d || {};
-  var opts = o || {};
+  var data = d || createObj(null);
+  var opts = o || createObj(null);
 
   // No options object -- if there are optiony names
   // in the data, copy them to options
@@ -483,7 +496,7 @@ exports.renderFile = function () {
     opts.filename = filename;
   }
   else {
-    data = {};
+    data = createObj(null);
   }
 
   return tryHandleCache(opts, data, cb);
@@ -505,8 +518,8 @@ exports.clearCache = function () {
 };
 
 function Template(text, opts) {
-  opts = opts || {};
-  var options = {};
+  opts = opts || createObj(null);
+  var options = createObj(null);
   this.templateText = text;
   /** @type {string | null} */
   this.mode = null;
@@ -587,12 +600,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  ';
           }
@@ -683,13 +705,14 @@ Template.prototype = {
     // Adds a local `include` function which allows full recursive include
     var returnedFn = opts.client ? fn : function anonymous(data) {
       var include = function (path, includeData) {
-        var d = utils.shallowCopy({}, data);
+        var d = utils.shallowCopy(createObj(null), data);
         if (includeData) {
           d = utils.shallowCopy(d, includeData);
         }
         return includeFile(path, opts)(d);
       };
-      return fn.apply(opts.context, [data || {}, escapeFn, include, rethrow]);
+      return fn.apply(opts.context,
+          [data || createObj(null), escapeFn, include, rethrow]);
     };
     if (opts.filename && typeof Object.defineProperty === 'function') {
       var filename = opts.filename;

diff --git a/test/ejs.js b/test/ejs.js
@@ -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/)
+  });
+});