Avoid prototype's values in _.template options

lodash · alexbrasetvik · Oct 16, 2019 · Oct 16, 2019 · Oct 16, 2019 · 19ad6bc39f05bbb4fba9edd56d83f1bc402ed87d
commit 19ad6bc39f05bbb4fba9edd56d83f1bc402ed87d
diff --git a/lodash.js b/lodash.js
@@ -1139,7 +1139,9 @@
   * @returns {*} Returns the property value if it's the object's own property.
   */
  function getOwnValue(object, key) {
-    return object == null ? undefined : (hasOwnProperty.call(object, key) && object[key] || undefined)
+    return object == null
+      ? undefined
+      : (hasOwnProperty.call(object, key) ? object[key] : undefined)
  }

  /**
@@ -14810,10 +14812,16 @@
        options = undefined;
      }
      string = toString(string);
+      /* These variables are particularly sensitive to prototype pollution.
+         Look them up prior to assignInWith, which will merge in anything
+         already in the prototype. */
+      var sourceURL = getOwnValue(options, 'sourceURL') || getOwnValue(settings, 'sourceURL'),
+          variable = getOwnValue(options, 'variable') || getOwnValue(settings, 'variable'),
+          imports = getOwnValue(options, 'imports') || getOwnValue(settings, 'imports');
+
      options = assignInWith({}, options, settings, customDefaultsAssignIn);

-      var imports = assignInWith({}, options.imports, settings.imports, customDefaultsAssignIn),
-          importsKeys = keys(imports),
+      var importsKeys = keys(imports),
          importsValues = baseValues(imports, importsKeys);

      var isEscaping,
@@ -14832,11 +14840,13 @@

      // Use a sourceURL for easier debugging.
      // The sourceURL gets injected into the source that's eval-ed, so be careful
-      // with lookup (in case of e.g. prototype pollution), and strip newlines if any.
-      // A newline wouldn't be a valid sourceURL anyway, and it'd enable code injection.
-      var sourceURL = '//# sourceURL=' +
-        (hasOwnProperty.call(options, 'sourceURL')
-          ? (options.sourceURL + '').replace(/[\r\n]/g, ' ')
+      // with lookup (in case of e.g. prototype pollution), and normalize all kinds of
+      // whitespace, so e.g. newlines (and unicode versions of it) can't sneak in.
+      // While variable is also injected below, it is less likely to be sourced from
+      // somewhere not trustworthy.
+      sourceURL = '//# sourceURL=' + (
+        sourceURL
+          ? (sourceURL + '').replace(/[\s]/g, ' ')
          : ('lodash.templateSources[' + (++templateCounter) + ']')
        ) + '\n';

@@ -14869,9 +14879,6 @@

      // If `variable` is not specified wrap a with-statement around the generated
      // code to add the data object to the top of the scope chain.
-      // Like with sourceURL, we take care to not check the option's prototype,
-      // as this configuration is a code injection vector.
-      var variable = hasOwnProperty.call(options, 'variable') && options.variable;
      if (!variable) {
        source = 'with (obj) {\n' + source + '\n}\n';
      }

diff --git a/test/test.js b/test/test.js
@@ -22652,6 +22652,27 @@
      assert.deepEqual(actual, expected);
    });

+    QUnit.test('should not let a polluted prototype affect generated code', function(assert) {
+      assert.expect(1);
+
+      // Intentionally pollute the prototype. These will cause a compilation error if part of the code
+      Object.prototype['sourceURL'] = '\u2028\n!err, please!';
+      Object.prototype['variable'] = '}{!?';
+      Object.prototype['imports'] = {',),': ''};
+
+      var actual,
+          expected = 'no error';
+      try {
+        actual = _.template(expected)();
+      } catch (e) {}
+
+      delete Object.prototype['sourceURL'];
+      delete Object.prototype['variable'];
+      delete Object.prototype['imports'];
+
+      assert.equal(actual, expected);
+    });
+
    QUnit.test('should work as an iteratee for methods like `_.map`', function(assert) {
      assert.expect(1);