10000 Improve performance of `toNumber`, `trim` and `trimEnd` on large inpu… · lodash/lodash@c4847eb · GitHub
[go: up one dir, main page]
Skip to content

Commit c4847eb

Browse files
falsyvaluesbnjmnt4n
falsyvalues
authored and
bnjmnt4n
committed
Improve performance of toNumber, trim and trimEnd on large input strings
This prevents potential ReDoS attacks using `_.toNumber` and `_.trim*` as potential attack vectors. Closes #5065.
1 parent 3469357 commit c4847eb

File tree

2 files changed

+68
-7
lines changed

2 files changed

+68
-7
lines changed

‎lodash.js

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,11 @@
153153
var reRegExpChar = /[\\^$.*+?()[\]{}|]/g,
154154
reHasRegExpChar = RegExp(reRegExpChar.source);
155155

156-
/** Used to match leading and trailing whitespace. */
157-
var reTrim = /^\s+|\s+$/g,
158-
reTrimStart = /^\s+/,
159-
reTrimEnd = /\s+$/;
156+
/** Used to match leading whitespace. */
157+
var reTrimStart = /^\s+/;
158+
159+
/** Used to match a single whitespace character. */
160+
var reWhitespace = /\s/;
160161

161162
/** Used to match wrap detail comments. */
162163
var reWrapComment = /\{(?:\n\/\* \[wrapped with .+\] \*\/)?\n?/,
@@ -1006,6 +1007,19 @@
10061007
});
10071008
}
10081009

1010+
/**
1011+
* The base implementation of `_.trim`.
1012+
*
1013+
* @private
1014+
* @param {string} string The string to trim.
1015+
* @returns {string} Returns the trimmed string.
1016+
*/
1017+
function baseTrim(string) {
1018+
return string
1019+
? string.slice(0, trimmedEndIndex(string) + 1).replace(reTrimStart, '')
1020+
: string;
1021+
}
1022+
10091023
/**
10101024
* The base implementation of `_.unary` without support for storing metadata.
10111025
*
@@ -1339,6 +1353,21 @@
13391353
: asciiToArray(string);
13401354
}
13411355

1356+
/**
1357+
* Used by `_.trim` and `_.trimEnd` to get the index of the last non-whitespace
1358+
* character of `string`.
1359+
*
1360+
* @private
1361+
* @param {string} string The string to inspect.
1362+
* @returns {number} Returns the index of the last non-whitespace character.
1363+
*/
1364+
function trimmedEndIndex(string) {
1365+
var index = string.length;
1366+
1367+
while (index-- && reWhitespace.test(string.charAt(index))) {}
1368+
return index;
1369+
}
1370+
13421371
/**
13431372
* Used by `_.unescape` to convert HTML entities to characters.
13441373
*
@@ -12507,7 +12536,7 @@
1250712536
if (typeof value != 'string') {
1250812537
return value === 0 ? value : +value;
1250912538
}
12510-
value = value.replace(reTrim, '');
12539+
value = baseTrim(value);
1251112540
var isBinary = reIsBinary.test(value);
1251212541
return (isBinary || reIsOctal.test(value))
1251312542
? freeParseInt(value.slice(2), isBinary ? 2 : 8)
@@ -14998,7 +15027,7 @@
1499815027
function trim(string, chars, guard) {
1499915028
string = toString(string);
1500015029
if (string && (guard || chars === undefined)) {
15001-
return string.replace(reTrim, '');
15030+
return baseTrim(string);
1500215031
}
1500315032
if (!string || !(chars = baseToString(chars))) {
1500415033
return string;
@@ -15033,7 +15062,7 @@
1503315062
function trimEnd(string, chars, guard) {
1503415063
string = toString(string);
1503515064
if (string && (guard || chars === undefined)) {
15036-
return string.replace(reTrimEnd, '');
15065+
return string.slice(0, trimmedEndIndex(string) + 1);
1503715066
}
1503815067
if (!string || !(chars = baseToString(chars))) {
1503915068
return string;

‎test/test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23783,6 +23783,22 @@
2378323783

2378423784
assert.deepEqual(actual, expected);
2378523785
});
23786+
23787+
QUnit.test('`_.`' + methodName + '` should prevent ReDoS', function(assert) {
23788+
assert.expect(2);
23789+
23790+
var largeStrLen = 50000,
23791+
largeStr = '1' + lodashStable.repeat(' ', largeStrLen) + '1',
23792+
maxMs = 1000,
23793+
startTime = lodashStable.now();
23794+
23795+
assert.deepEqual(_[methodName](largeStr), methodName == 'toNumber' ? NaN : 0);
23796+
23797+
var endTime = lodashStable.now(),
23798+
timeSpent = endTime - startTime;
23799+
23800+
assert.ok(timeSpent < maxMs, 'operation took ' + timeSpent + 'ms');
23801+
});
2378623802
});
2378723803

2378823804
/*--------------------------------------------------------------------------*/
@@ -24368,6 +24384,22 @@
2436824384
assert.strictEqual(func(string, ''), string);
2436924385
});
2437024386

24387+
QUnit.test('`_.`' + methodName + '` should prevent ReDoS', function(assert) {
24388+
assert.expect(2);
24389+
24390+
var largeStrLen = 50000,
24391+
largeStr = 'A' + lodashStable.repeat(' ', largeStrLen) + 'A',
24392+
maxMs = 1000,
24393+
startTime = lodashStable.now();
24394+
24395+
assert.strictEqual(_[methodName](largeStr), largeStr);
24396+
24397+
var endTime = lodashStable.now(),
24398+
timeSpent = endTime - startTime;
24399+
24400+
assert.ok(timeSpent < maxMs, 'operation took ' + timeSpent + 'ms');
24401+
});
24402+
2437124403
QUnit.test('`_.' + methodName + '` should work as an iteratee for methods like `_.map`', function(assert) {
2437224404
assert.expect(1);
2437324405

0 commit comments

Comments
 (0)
0