Fixes ReDOS vulnerabilities.

Jamie Davis (@davisjam) from Virginia Tech reported that clean-css
suffers from ReDOS vulnerability [0] when fed with crafted input.

Since not so many people use clean-css allowing untrusted input such
cases may be rare, but this commit reworks vulnerable code to prevent
such attacks.

It also limits certain whitespace blocks to sane length of 31 characters
in validation regexes to prevent similar issues.

[0] https://snyk.io/blog/redos-and-catastrophic-backtracking

History.md

@@ -5,6 +5,7 @@
 * Fixed issue [#861](https://github.com/jakubpawlowicz/clean-css/issues/861) - new `transition` property optimizer.
 * Fixed issue [#895](https://github.com/jakubpawlowicz/clean-css/issues/895) - ignoring specific styles.
 * Fixed issue [#947](https://github.com/jakubpawlowicz/clean-css/issues/947) - selector based filtering.
+* Fixes ReDOS vulnerabilities in validator code.
 
 [4.1.10 / 2018-03-05](https://github.com/jakubpawlowicz/clean-css/compare/v4.1.9...v4.1.10)
 ==================

README.md

@@ -743,6 +743,7 @@ Sorted alphabetically by GitHub handle:
 * [@alexlamsl](https://github.com/alexlamsl) (Alex Lam S.L.) for testing early clean-css 4 versions, reporting bugs, and suggesting numerous improvements.
 * [@altschuler](https://github.com/altschuler) (Simon Altschuler) for fixing `@import` processing inside comments;
 * [@ben-eb](https://github.com/ben-eb) (Ben Briggs) for sharing ideas about CSS optimizations;
+* [@davisjam](https://github.com/davisjam) (Jamie Davis) for disclosing ReDOS vulnerabilities;
 * [@facelessuser](https://github.com/facelessuser) (Isaac) for pointing out a flaw in clean-css' stateless mode;
 * [@grandrath](https://github.com/grandrath) (Martin Grandrath) for improving `minify` method source traversal in ES6;
 * [@jmalonzo](https://github.com/jmalonzo) (Jan Michael Alonzo) for a patch removing node.js' old `sys` package;

lib/optimizer/level-2/can-override.js

@@ -199,6 +199,24 @@ function unitOrKeywordWithGlobal(propertyName) {
   };
 }
 
+function unitOrNumber(validator, value1, value2) {
+  if (!understandable(validator, value1, value2, 0, true) && !(validator.isUnit(value2) || validator.isNumber(value2))) {
+    return false;
+  } else if (validator.isVariable(value1) && validator.isVariable(value2)) {
+    return true;
+  } else if ((validator.isUnit(value1) || validator.isNumber(value1)) && !(validator.isUnit(value2) || validator.isNumber(value2))) {
+    return false;
+  } else if (validator.isUnit(value2) || validator.isNumber(value2)) {
+    return true;
+  } else if (validator.isUnit(value1) || validator.isNumber(value1)) {
+    return false;
+  } else if (validator.isFunction(value1) && !validator.isPrefixed(value1) && validator.isFunction(value2) && !validator.isPrefixed(value2)) {
+    return true;
+  }
+
+  return sameFunctionOrValue(validator, value1, value2);
+}
+
 function zIndex(validator, value1, value2) {
   if (!understandable(validator, value1, value2, 0, true) && !validator.isZIndex(value2)) {
     return false;
@@ -217,7 +235,8 @@ module.exports = {
     propertyName: propertyName,
     time: time,
     timingFunction: timingFunction,
-    unit: unit
+    unit: unit,
+    unitOrNumber: unitOrNumber
   },
   property: {
     animationDirection: keywordWithGlobal('animation-direction'),

lib/optimizer/level-2/compactable.js

@@ -681,7 +681,7 @@ var compactable = {
     defaultValue: 'auto'
   },
   'line-height': {
-    canOverride: canOverride.generic.unit,
+    canOverride: canOverride.generic.unitOrNumber,
     defaultValue: 'normal',
     shortestValue: '0'
   },

lib/optimizer/level-2/remove-unused-at-rules.js

@@ -8,7 +8,7 @@ var Token = require('../../tokenizer/token');
 var animationNameRegex = /^(\-moz\-|\-o\-|\-webkit\-)?animation-name$/;
 var animationRegex = /^(\-moz\-|\-o\-|\-webkit\-)?animation$/;
 var keyframeRegex = /^@(\-moz\-|\-o\-|\-webkit\-)?keyframes /;
-var importantRegex = /\s*!important$/;
+var importantRegex = /\s{0,31}!important$/;
 var optionalMatchingQuotesRegex = /^(['"]?)(.*)\1$/;
 
 function normalize(value) {

lib/optimizer/validator.js

@@ -4,19 +4,24 @@ var variableRegexStr = 'var\\(\\-\\-[^\\)]+\\)';
 var functionAnyRegexStr = '(' + variableRegexStr + '|' + functionNoVendorRegexStr + '|' + functionVendorRegexStr + ')';
 
 var calcRegex = new RegExp('^(\\-moz\\-|\\-webkit\\-)?calc\\([^\\)]+\\)$', 'i');
+var decimalRegex = /[0-9]/;
 var functionAnyRegex = new RegExp('^' + functionAnyRegexStr + '$', 'i');
-var hslColorRegex = /^hsl\(\s*[\-\.\d]+\s*,\s*[\.\d]+%\s*,\s*[\.\d]+%\s*\)|hsla\(\s*[\-\.\d]+\s*,\s*[\.\d]+%\s*,\s*[\.\d]+%\s*,\s*[\.\d]+\s*\)$/;
+var hslColorRegex = /^hsl\(\s{0,31}[\-\.]?\d+\s{0,31},\s{0,31}\.?\d+%\s{0,31},\s{0,31}\.?\d+%\s{0,31}\)|hsla\(\s{0,31}[\-\.]?\d+\s{0,31},\s{0,31}\.?\d+%\s{0,31},\s{0,31}\.?\d+%\s{0,31},\s{0,31}\.?\d+\s{0,31}\)$/;
 var identifierRegex = /^(\-[a-z0-9_][a-z0-9\-_]*|[a-z][a-z0-9\-_]*)$/i;
 var longHexColorRegex = /^#[0-9a-f]{6}$/i;
 var namedEntityRegex = /^[a-z]+$/i;
 var prefixRegex = /^-([a-z0-9]|-)*$/i;
-var rgbColorRegex = /^rgb\(\s*[\d]{1,3}\s*,\s*[\d]{1,3}\s*,\s*[\d]{1,3}\s*\)|rgba\(\s*[\d]{1,3}\s*,\s*[\d]{1,3}\s*,\s*[\d]{1,3}\s*,\s*[\.\d]+\s*\)$/;
+var rgbColorRegex = /^rgb\(\s{0,31}[\d]{1,3}\s{0,31},\s{0,31}[\d]{1,3}\s{0,31},\s{0,31}[\d]{1,3}\s{0,31}\)|rgba\(\s{0,31}[\d]{1,3}\s{0,31},\s{0,31}[\d]{1,3}\s{0,31},\s{0,31}[\d]{1,3}\s{0,31},\s{0,31}[\.\d]+\s{0,31}\)$/;
 var shortHexColorRegex = /^#[0-9a-f]{3}$/i;
-var timeRegex = new RegExp('^(\\-?\\+?\\.?\\d+\\.?\\d*(s|ms))$');
 var timingFunctionRegex = /^(cubic\-bezier|steps)\([^\)]+\)$/;
+var validTimeUnits = ['ms', 's'];
 var urlRegex = /^url\([\s\S]+\)$/i;
 var variableRegex = new RegExp('^' + variableRegexStr + '$', 'i');
 
+var DECIMAL_DOT = '.';
+var MINUS_SIGN = '-';
+var PLUS_SIGN = '+';
+
 var Keywords = {
   '^': [
     'inherit',
@@ -386,7 +391,7 @@ function isNamedEntity(value) {
 }
 
 function isNumber(value) {
-  return value.length > 0 && ('' + parseFloat(value)) === value;
+  return scanForNumber(value) == value.length;
 }
 
 function isRgbColor(value) {
@@ -407,7 +412,10 @@ function isVariable(value) {
 }
 
 function isTime(value) {
-  return timeRegex.test(value);
+  var numberUpTo = scanForNumber(value);
+
+  return numberUpTo == value.length && parseInt(value) === 0 ||
+    numberUpTo > -1 && validTimeUnits.indexOf(value.slice(numberUpTo + 1)) > -1;
 }
 
 function isTimingFunction() {
@@ -418,8 +426,13 @@ function isTimingFunction() {
   };
 }
 
-function isUnit(compatibleCssUnitRegex, value) {
-  return compatibleCssUnitRegex.test(value);
+function isUnit(validUnits, value) {
+  var numberUpTo = scanForNumber(value);
+
+  return numberUpTo == value.length && parseInt(value) === 0 ||
+    numberUpTo > -1 && validUnits.indexOf(value.slice(numberUpTo + 1)) > -1 ||
+    value == 'auto' ||
+    value == 'inherit';
 }
 
 function isUrl(value) {
@@ -432,13 +445,38 @@ function isZIndex(value) {
     isKeyword('^')(value);
 }
 
+function scanForNumber(value) {
+  var hasDot = false;
+  var hasSign = false;
+  var character;
+  var i, l;
+
+  for (i = 0, l = value.length; i < l; i++) {
+    character = value[i];
+
+    if (i === 0 && (character == PLUS_SIGN || character == MINUS_SIGN)) {
+      hasSign = true;
+    } else if (i > 0 && hasSign && (character == PLUS_SIGN || character == MINUS_SIGN)) {
+      return i - 1;
+    } else if (character == DECIMAL_DOT && !hasDot) {
+      hasDot = true;
+    } else if (character == DECIMAL_DOT && hasDot) {
+      return i - 1;
+    } else if (decimalRegex.test(character)) {
+      continue;
+    } else {
+      return i - 1;
+    }
+  }
+
+  return i;
+}
+
 function validator(compatibility) {
   var validUnits = Units.slice(0).filter(function (value) {
     return !(value in compatibility.units) || compatibility.units[value] === true;
   });
 
-  var compatibleCssUnitRegex = new RegExp('^(\\-?\\.?\\d+\\.?\\d*(' + validUnits.join('|') + '|)|auto|inherit)$', 'i');
-
   return {
     colorOpacity: compatibility.colors.opacity,
     isAnimationDirectionKeyword: isKeyword('animation-direction'),
@@ -471,12 +509,13 @@ function validator(compatibility) {
     isLineHeightKeyword: isKeyword('line-height'),
     isListStylePositionKeyword: isKeyword('list-style-position'),
     isListStyleTypeKeyword: isKeyword('list-style-type'),
+    isNumber: isNumber,
     isPrefixed: isPrefixed,
     isPositiveNumber: isPositiveNumber,
     isRgbColor: isRgbColor,
     isStyleKeyword: isKeyword('*-style'),
     isTime: isTime,
-    isUnit: isUnit.bind(null, compatibleCssUnitRegex),
+    isUnit: isUnit.bind(null, validUnits),
     isUrl: isUrl,
     isVariable: isVariable,
     isWidth: isKeyword('width'),

lib/tokenizer/tokenize.js

@@ -59,7 +59,7 @@ var EXTRA_PAGE_BOXES = [
   '@right'
 ];
 
-var REPEAT_PATTERN = /^\[\s*\d+\s*\]$/;
+var REPEAT_PATTERN = /^\[\s{0,31}\d+\s{0,31}\]$/;
 var RULE_WORD_SEPARATOR_PATTERN = /[\s\(]/;
 var TAIL_BROKEN_VALUE_PATTERN = /[\s|\}]*$/;
 

test/module-test.js

@@ -869,5 +869,27 @@ vows.describe('module tests').addBatch({
     'should raise no errors': function (error, minified) {
       assert.isEmpty(minified.errors);
     }
+  },
+  'vulnerabilities': {
+    'ReDOS in time units': {
+      'topic': function () {
+        var prefix = '-+.0';
+        var pump = [];
+        var suffix = '-0';
+        var input;
+        var i;
+
+        for (i = 0; i < 10000; i++) {
+          pump.push('0000000000');
+        }
+
+        input = '.block{animation:1s test;animation-duration:' + prefix + pump.join('') + suffix + 's}';
+
+        return new CleanCSS({ level: { 1: { replaceZeroUnits: false }, 2: true } }).minify(input);
+      },
+      'finishes in less than a second': function (error, minified) {
+        assert.isTrue(minified.stats.timeSpent < 1000);
+      }
+    }
   }
 }).export(module);