Merge pull request #556 from chriso/chriso/trim-redos

Fix a regex denial of service in trim() and rtrim()

lib/rtrim.js

@@ -13,7 +13,13 @@ function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { de
 
 function rtrim(str, chars) {
   (0, _assertString2.default)(str);
-  var pattern = chars ? new RegExp('[' + chars + ']+$', 'g') : /\s+$/g;
-  return str.replace(pattern, '');
+  var pattern = chars ? new RegExp('[' + chars + ']') : /\s/;
+
+  var idx = str.length - 1;
+  while (idx >= 0 && pattern.test(str[idx])) {
+    idx--;
+  }
+
+  return idx < str.length ? str.substr(0, idx + 1) : str;
 }
 module.exports = exports['default'];

lib/trim.js

@@ -5,15 +5,17 @@ Object.defineProperty(exports, "__esModule", {
 });
 exports.default = trim;
 
-var _assertString = require('./util/assertString');
+var _rtrim = require('./rtrim');
 
-var _assertString2 = _interopRequireDefault(_assertString);
+var _rtrim2 = _interopRequireDefault(_rtrim);
+
+var _ltrim = require('./ltrim');
+
+var _ltrim2 = _interopRequireDefault(_ltrim);
 
 function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }
 
 function trim(str, chars) {
-  (0, _assertString2.default)(str);
-  var pattern = chars ? new RegExp('^[' + chars + ']+|[' + chars + ']+$', 'g') : /^\s+|\s+$/g;
-  return str.replace(pattern, '');
+  return (0, _rtrim2.default)((0, _ltrim2.default)(str, chars), chars);
 }
 module.exports = exports['default'];

src/lib/rtrim.js

@@ -2,6 +2,12 @@ import assertString from './util/assertString';
 
 export default function rtrim(str, chars) {
   assertString(str);
-  const pattern = chars ? new RegExp(`[${chars}]+$`, 'g') : /\s+$/g;
-  return str.replace(pattern, '');
+  const pattern = chars ? new RegExp(`[${chars}]`) : /\s/;
+
+  let idx = str.length - 1;
+  while (idx >= 0 && pattern.test(str[idx])) {
+    idx--;
+  }
+
+  return idx < str.length ? str.substr(0, idx + 1) : str;
 }

src/lib/trim.js

@@ -1,7 +1,6 @@
-import assertString from './util/assertString';
+import rtrim from './rtrim';
+import ltrim from './ltrim';
 
 export default function trim(str, chars) {
-  assertString(str);
-  const pattern = chars ? new RegExp(`^[${chars}]+|[${chars}]+$`, 'g') : /^\s+|\s+$/g;
-  return str.replace(pattern, '');
+  return rtrim(ltrim(str, chars), chars);
 }

test/sanitizers.js

@@ -53,17 +53,26 @@ describe('Sanitizers', function () {
   it('should trim whitespace', function () {
     test({
       sanitizer: 'trim',
-      expect: { '  \r\n\tfoo  \r\n\t   ': 'foo' },
+      expect: {
+        '  \r\n\tfoo  \r\n\t   ': 'foo',
+        '      \r': '',
+      },
     });
 
     test({
       sanitizer: 'ltrim',
-      expect: { '  \r\n\tfoo  \r\n\t   ': 'foo  \r\n\t   ' },
+      expect: {
+        '  \r\n\tfoo  \r\n\t   ': 'foo  \r\n\t   ',
+        '   \t  \n': '',
+      },
     });
 
     test({
       sanitizer: 'rtrim',
-      expect: { '  \r\n\tfoo  \r\n\t   ': '  \r\n\tfoo' },
+      expect: {
+        '  \r\n\tfoo  \r\n\t   ': '  \r\n\tfoo',
+        ' \r\n  \t': '',
+      },
     });
   });
 

validator.js

@@ -1001,14 +1001,18 @@
 
       function rtrim(str, chars) {
         assertString(str);
-        var pattern = chars ? new RegExp('[' + chars + ']+$', 'g') : /\s+$/g;
-        return str.replace(pattern, '');
+        var pattern = chars ? new RegExp('[' + chars + ']') : /\s/;
+
+        var idx = str.length - 1;
+        while (idx >= 0 && pattern.test(str[idx])) {
+          idx--;
+        }
+
+        return idx < str.length ? str.substr(0, idx + 1) : str;
       }
 
       function trim(str, chars) {
-        assertString(str);
-        var pattern = chars ? new RegExp('^[' + chars + ']+|[' + chars + ']+$', 'g') : /^\s+|\s+$/g;
-        return str.replace(pattern, '');
+        return rtrim(ltrim(str, chars), chars);
       }
 
       function escape(str) {