Make parsing of <transform> more strict

third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/parser/CSSPropertyParser.h"
 
 #include "core/StylePropertyShorthand.h"
 #include "core/css/CSSBasicShapeValues.h"
 #include "core/css/CSSBorderImage.h"
 #include "core/css/CSSContentDistributionValue.h"
@@ skipping 1789 matching lines @@
     case CSSValueRotateZ:
     case CSSValueSkewX:
     case CSSValueSkewY:
     case CSSValueSkew:
         parsedValue = consumeAngle(args);
         if (!parsedValue)
             return nullptr;
         if (functionId == CSSValueSkew && consumeCommaIncludingWhitespace(args)) {
             transformValue->append(parsedValue);
             parsedValue = consumeAngle(args);
+            if (!parsedValue)
+                return nullptr;
         }
         break;
     case CSSValueScaleX:
     case CSSValueScaleY:
     case CSSValueScaleZ:
     case CSSValueScale:
         parsedValue = consumeNumber(args, ValueRangeAll);
         if (!parsedValue)
             return nullptr;
         if (functionId == CSSValueScale && consumeCommaIncludingWhitespace(args)) {
             transformValue->append(parsedValue);
             parsedValue = consumeNumber(args, ValueRangeAll);
+            if (!parsedValue)
+                return nullptr;
         }
         break;
     case CSSValuePerspective:
         if (!consumePerspective(args, cssParserMode, transformValue, useLegacyParsing))
             return nullptr;
         break;
     case CSSValueTranslateX:
     case CSSValueTranslateY:
     case CSSValueTranslate:
         parsedValue = consumeLengthOrPercent(args, cssParserMode, ValueRangeAll);
         if (!parsedValue)
             return nullptr;
         if (functionId == CSSValueTranslate && consumeCommaIncludingWhitespace(args)) {
             transformValue->append(parsedValue);
             parsedValue = consumeLengthOrPercent(args, cssParserMode, ValueRangeAll);
+            if (!parsedValue)
+                return nullptr;
         }
         break;
     case CSSValueTranslateZ:
         parsedValue = consumeLength(args, cssParserMode, ValueRangeAll);

[Timothy Loh, 2016/03/17 23:23:32]

Is this case right? Maybe it'd be better to make the cases which call out to
e.g. consumeNumbers always return, while the bottom of the switch returns
nullptr if we didn't set parsedValue?

[rwlbuis, 2016/03/17 23:29:48]

This should be correct, since the no args case is handled above, and any failure
to consume
a length should cause the atEnd() check at the bottom of this method to fail.
There should also already be tests for this (at least the empty args one).

[Timothy Loh, 2016/03/17 23:56:45]

Ah right. lgtm then, I just realised my suggestion doesn't work too well because
the cases which don't use parsedValue still need to check atEnd...

         break;
     case CSSValueMatrix:
     case CSSValueMatrix3d:
         if (!consumeNumbers(args, transformValue, (functionId == CSSValueMatrix3d) ? 16 : 6))
             return nullptr;
         break;
     case CSSValueScale3d:
         if (!consumeNumbers(args, transformValue, 3))
             return nullptr;
         break;
     case CSSValueRotate3d:
         if (!consumeNumbers(args, transformValue, 3) || !consumeCommaIncludingWhitespace(args))
             return nullptr;
         parsedValue = consumeAngle(args);
+        if (!parsedValue)
+            return nullptr;
         break;
     case CSSValueTranslate3d:
         if (!consumeTranslate3d(args, cssParserMode, transformValue))
             return nullptr;
         break;
     default:
         return nullptr;
     }
     if (parsedValue)
         transformValue->append(parsedValue);
@@ skipping 2765 matching lines @@
         m_currentShorthand = oldShorthand;
         CSSParserValueList valueList(m_range);
         if (!valueList.size())
             return false;
         m_valueList = &valueList;
         return legacyParseShorthand(unresolvedProperty, important);
     }
 }
 
 } // namespace blink