Improve performance of toNumber, trim and trimEnd on large input strings

falsyvalues · bnjmnt4n · commit c4847ebe7d14 · 2021-02-20T17:18:39.000+08:00
This prevents potential ReDoS attacks using `_.toNumber` and `_.trim*` as potential attack vectors. Closes #5065.
diff --git a/lodash.js b/lodash.js
@@ -153,10 +153,11 @@
   var reRegExpChar = /[\\^$.*+?()[\]{}|]/g,
       reHasRegExpChar = RegExp(reRegExpChar.source);
 
-  /** Used to match leading and trailing whitespace. */
-  var reTrim = /^\s+|\s+$/g,
-      reTrimStart = /^\s+/,
-      reTrimEnd = /\s+$/;
+  /** Used to match leading whitespace. */
+  var reTrimStart = /^\s+/;
+
+  /** Used to match a single whitespace character. */
+  var reWhitespace = /\s/;
 
   /** Used to match wrap detail comments. */
   var reWrapComment = /\{(?:\n\/\* \[wrapped with .+\] \*\/)?\n?/,
@@ -1006,6 +1007,19 @@
     });
   }
 
+  /**
+   * The base implementation of `_.trim`.
+   *
+   * @private
+   * @param {string} string The string to trim.
+   * @returns {string} Returns the trimmed string.
+   */
+  function baseTrim(string) {
+    return string
+      ? string.slice(0, trimmedEndIndex(string) + 1).replace(reTrimStart, '')
+      : string;
+  }
+
   /**
    * The base implementation of `_.unary` without support for storing metadata.
    *
@@ -1339,6 +1353,21 @@
       : asciiToArray(string);
   }
 
+  /**
+   * Used by `_.trim` and `_.trimEnd` to get the index of the last non-whitespace
+   * character of `string`.
+   *
+   * @private
+   * @param {string} string The string to inspect.
+   * @returns {number} Returns the index of the last non-whitespace character.
+   */
+  function trimmedEndIndex(string) {
+    var index = string.length;
+
+    while (index-- && reWhitespace.test(string.charAt(index))) {}
+    return index;
+  }
+
   /**
    * Used by `_.unescape` to convert HTML entities to characters.
    *
@@ -12507,7 +12536,7 @@
       if (typeof value != 'string') {
         return value === 0 ? value : +value;
       }
-      value = value.replace(reTrim, '');
+      value = baseTrim(value);
       var isBinary = reIsBinary.test(value);
       return (isBinary || reIsOctal.test(value))
         ? freeParseInt(value.slice(2), isBinary ? 2 : 8)
@@ -14998,7 +15027,7 @@
     function trim(string, chars, guard) {
       string = toString(string);
       if (string && (guard || chars === undefined)) {
-        return string.replace(reTrim, '');
+        return baseTrim(string);
       }
       if (!string || !(chars = baseToString(chars))) {
         return string;
@@ -15033,7 +15062,7 @@
     function trimEnd(string, chars, guard) {
       string = toString(string);
       if (string && (guard || chars === undefined)) {
-        return string.replace(reTrimEnd, '');
+        return string.slice(0, trimmedEndIndex(string) + 1);
       }
       if (!string || !(chars = baseToString(chars))) {
         return string;
diff --git a/test/test.js b/test/test.js
@@ -23783,6 +23783,22 @@
 
       assert.deepEqual(actual, expected);
     });
+
+    QUnit.test('`_.`' + methodName + '` should prevent ReDoS', function(assert) {
+      assert.expect(2);
+
+      var largeStrLen = 50000,
+          largeStr = '1' + lodashStable.repeat(' ', largeStrLen) + '1',
+          maxMs = 1000,
+          startTime = lodashStable.now();
+
+      assert.deepEqual(_[methodName](largeStr), methodName == 'toNumber' ? NaN : 0);
+
+      var endTime = lodashStable.now(),
+          timeSpent = endTime - startTime;
+
+      assert.ok(timeSpent < maxMs, 'operation took ' + timeSpent + 'ms');
+    });
   });
 
   /*--------------------------------------------------------------------------*/
@@ -24368,6 +24384,22 @@
       assert.strictEqual(func(string, ''), string);
     });
 
+    QUnit.test('`_.`' + methodName + '` should prevent ReDoS', function(assert) {
+      assert.expect(2);
+
+      var largeStrLen = 50000,
+          largeStr = 'A' + lodashStable.repeat(' ', largeStrLen) + 'A',
+          maxMs = 1000,
+          startTime = lodashStable.now();
+
+      assert.strictEqual(_[methodName](largeStr), largeStr);
+
+      var endTime = lodashStable.now(),
+          timeSpent = endTime - startTime;
+
+      assert.ok(timeSpent < maxMs, 'operation took ' + timeSpent + 'ms');
+    });
+
     QUnit.test('`_.' + methodName + '` should work as an iteratee for methods like `_.map`', function(assert) {
       assert.expect(1);
 
