CVE-2017-1000188: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: nodejs ejs version older than 2.5.5 is vulnerable to a Cross-site-scripting in the ejs.renderFile() resulting in code injection
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2017-11-17T03:29Z
Repository: https://github.com/mde/ejs
Language: JavaScript
Files changed: lib/ejs.js, test/ejs.js
Commit date: 2016-12-01 16:21:47 -0800
Commit message: Blacklist a few other unsafe opts from passing in data obj

Diff: diff --git a/lib/ejs.js b/lib/ejs.js
index 98e5991c..04b9bcb6 100644
--- a/lib/ejs.js
+++ b/lib/ejs.js
@@ -54,10 +54,16 @@ var _DEFAULT_DELIMITER = '%';
 var _DEFAULT_LOCALS_NAME = 'locals';
 var _REGEX_STRING = '(<%%|%%>|<%=|<%-|<%_|<%#|<%|%>|-%>|_%>)';
 var _OPTS = [ 'cache', 'filename', 'delimiter', 'scope', 'context',
         'debug', 'compileDebug', 'client', '_with', 'root', 'rmWhitespace',
         'strict', 'localsName'];
+var _OPTS_IN_DATA_BLACKLIST = {
+      cache: true,
+      filename: true,
+      root: true,
+      localsName: true
+    };
 var _BOM = /^\uFEFF/;
 
 /**
  * EJS template function cache. This can be a LRU object from lru-cache NPM
  * module. By default, it is {@link module:utils.cache}, a simple in-process
@@ -266,15 +272,13 @@ function rethrow(err, str, filename, lineno){
  */
 
 function cpOptsInData(data, opts) {
   _OPTS.forEach(function (p) {
     if (typeof data[p] != 'undefined') {
-      // Disallow setting the root opt for includes via a passed data obj
-      // Unsanitized, parameterized use of `render` could allow the
-      // include directory to be reset, opening up the possibility of
-      // remote code execution
-      if (p == 'root') {
+      // Disallow passing potentially dangerous opts in the data
+      // These opts should not be settable via a `render` call
+      if (_OPTS_IN_DATA_BLACKLIST[p]) {
         return;
       }
       opts[p] = data[p];
     }
   });
diff --git a/test/ejs.js b/test/ejs.js
index c2bb87a7..b8cb4c9d 100644
--- a/test/ejs.js
+++ b/test/ejs.js
@@ -166,10 +166,25 @@ suite('ejs.compile(str, options)', function () {
     assert.equal(ejs.render(fixture('strict.ejs'), {}, {strict: true}), 'true');
   });
 
 });
 
+/* Old API -- remove when this shim goes away */
+suite('ejs.render(str, dataAndOpts)', function () {
+  test('render the template with data/opts passed together', function () {
+    assert.equal(ejs.render('<p><?= foo ?></p>', {foo: 'yay', delimiter: '?'}),
+        '<p>yay</p>');
+  });
+
+  test('disallow unsafe opts passed along in data', function () {
+    assert.equal(ejs.render('<p><?= locals.foo ?></p>',
+        // localsName should not get reset because it's blacklisted
+        {_with: false, foo: 'yay', delimiter: '?', localsName: '_'}),
+        '<p>yay</p>');
+  });
+});
+
 suite('ejs.render(str, data, opts)', function () {
   test('render the template', function () {
     assert.equal(ejs.render('<p>yay</p>'), '<p>yay</p>');
   });
 
@@ -751,11 +766,10 @@ suite('include()', function () {
   test('include ejs with set root path', function () {
     var file = 'test/fixtures/include-root.ejs';
     var viewsPath = path.join(__dirname, 'fixtures');
     assert.equal(ejs.render(fixture('include-root.ejs'), {pets: users}, {filename: file, delimiter: '@',root:viewsPath}),
       fixture('include.html'));
-
   });
 
   test('work when nested', function () {
     var file = 'test/fixtures/menu.ejs';
     assert.equal(ejs.render(fixture('menu.ejs'), {pets: users}, {filename: file}),
@@ -916,11 +930,11 @@ suite('preprocessor include', function () {
   test('whitespace slurp and rmWhitespace work', function() {
     var file = 'test/fixtures/include_preprocessor_line_slurp.ejs';
     var template = fixture('include_preprocessor_line_slurp.ejs');
     var expected = fixture('include_preprocessor_line_slurp.html');
     var options = {rmWhitespace: true, filename: file};
-    assert.equal(ejs.render(template, options),
+    assert.equal(ejs.render(template, {}, options),
         expected);
   });
 
 });
 

Vulnerable code:       // Disallow setting the root opt for includes via a passed data obj
      // Unsanitized, parameterized use of `render` could allow the
      // include directory to be reset, opening up the possibility of
      // remote code execution
      if (p == 'root') {

    assert.equal(ejs.render(template, options),
Fixed code: var _OPTS_IN_DATA_BLACKLIST = {
      cache: true,
      filename: true,
      root: true,
      localsName: true
    };
      // Disallow passing potentially dangerous opts in the data
      // These opts should not be settable via a `render` call
      if (_OPTS_IN_DATA_BLACKLIST[p]) {
/* Old API -- remove when this shim goes away */
suite('ejs.render(str, dataAndOpts)', function () {
  test('render the template with data/opts passed together', function () {
    assert.equal(ejs.render('<p><?= foo ?></p>', {foo: 'yay', delimiter: '?'}),
        '<p>yay</p>');
  });

  test('disallow unsafe opts passed along in data', function () {
    assert.equal(ejs.render('<p><?= locals.foo ?></p>',
        // localsName should not get reset because it's blacklisted
        {_with: false, foo: 'yay', delimiter: '?', localsName: '_'}),
        '<p>yay</p>');
  });
});

    assert.equal(ejs.render(template, {}, options),