WebCore:
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Mar 2008 06:46:54 +0000 (06:46 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 26 Mar 2008 06:46:54 +0000 (06:46 +0000)
2008-03-25  Beth Dakin  <bdakin@apple.com>

        Reviewed by Oliver.

        Fix for <rdar://problem/5811826> CSSValueList::item() does not
        range-check index

        Check bounds before accessing the item to avoid a crash.
        itemWithoutBoundsCheck() is still inlined and not bounds-checked to
        avoid slowing down our internal callers of item().
        * css/CSSValueList.cpp:
        (WebCore::CSSValueList::item):
        * css/CSSValueList.h:
        (WebCore::CSSValueList::itemWithoutBoundsCheck):

        Call itemWithoutBoundsCheck() to avoid slowing down these internal
        callers.
        * css/CSSFontSelector.cpp:
        (WebCore::CSSFontSelector::addFontFaceRule):
        * css/CSSMutableStyleDeclaration.cpp:
        (WebCore::CSSMutableStyleDeclaration::getLayeredShorthandValue):
        * css/CSSStyleSelector.cpp:
        (WebCore::applyCounterList):
        (WebCore::CSSStyleSelector::applyProperty):
        * css/MediaQueryEvaluator.cpp:
        (WebCore::parseAspectRatio):
        * svg/SVGFontFaceElement.cpp:
        (WebCore::SVGFontFaceElement::rebuildFontFace):
        * svg/graphics/SVGPaintServer.cpp:
        (WebCore::dashArrayFromRenderingStyle):

LayoutTests:

2008-03-25  Beth Dakin  <bdakin@apple.com>

        Reviewed by Oliver.

        Test for <rdar://problem/5811826> CSSValueList::item() does not
        range-check index

        * fast/css/resources/bikes.bmp: Added.
        * fast/css/value-list-out-of-bounds-crash.html: Added.
        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.checksum: Added.
        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png: Added.
        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.txt: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31309 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css/resources/bikes.bmp [new file with mode: 0644]
LayoutTests/fast/css/value-list-out-of-bounds-crash.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/css/CSSFontSelector.cpp
WebCore/css/CSSMutableStyleDeclaration.cpp
WebCore/css/CSSStyleSelector.cpp
WebCore/css/CSSValueList.cpp
WebCore/css/CSSValueList.h
WebCore/css/MediaQueryEvaluator.cpp
WebCore/svg/SVGFontFaceElement.cpp
WebCore/svg/graphics/SVGPaintServer.cpp

index aad495ee83914330c944a67b628f741df159766c..d8c754508f3d95e33637c94dfe086c2830482098 100644 (file)
@@ -1,3 +1,16 @@
+2008-03-25  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Oliver.
+
+        Test for <rdar://problem/5811826> CSSValueList::item() does not 
+        range-check index
+
+        * fast/css/resources/bikes.bmp: Added.
+        * fast/css/value-list-out-of-bounds-crash.html: Added.
+        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.checksum: Added.
+        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png: Added.
+        * platform/mac/fast/css/value-list-out-of-bounds-crash-expected.txt: Added.
+
 2008-03-25  Darin Adler  <darin@apple.com>
 
         Reviewed by Anders.
diff --git a/LayoutTests/fast/css/resources/bikes.bmp b/LayoutTests/fast/css/resources/bikes.bmp
new file mode 100644 (file)
index 0000000..a60c001
Binary files /dev/null and b/LayoutTests/fast/css/resources/bikes.bmp differ
diff --git a/LayoutTests/fast/css/value-list-out-of-bounds-crash.html b/LayoutTests/fast/css/value-list-out-of-bounds-crash.html
new file mode 100644 (file)
index 0000000..0067b76
--- /dev/null
@@ -0,0 +1,26 @@
+<head>
+<style>
+    #div {
+        width: 200px;
+        height: 200px;
+        background-image:url(resources/bikes.bmp); 
+        -webkit-background-size: 50% auto;
+    }
+</style>
+</head>
+
+<body>
+
+<div id='pass'>Failed! This test should not crash, and this text should be changed to a passing message.</div>
+<div id='div'></div>
+
+<script>
+    var div = document.getElementById('div');
+    var style = document.defaultView.getComputedStyle(div);
+    var valueList = style.getPropertyCSSValue('-webkit-background-size');
+    var one = valueList.item(100);
+
+    var pass = document.getElementById('pass');
+    pass.innerHTML = 'PASS! This test passes if it does not crash.';
+</script>
+</body>
diff --git a/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.checksum b/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.checksum
new file mode 100644 (file)
index 0000000..066a589
--- /dev/null
@@ -0,0 +1 @@
+5ed2721a1201b3d97fb5a490b55ff0b6
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png b/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png
new file mode 100644 (file)
index 0000000..3a788a1
Binary files /dev/null and b/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.png differ
diff --git a/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.txt b/LayoutTests/platform/mac/fast/css/value-list-out-of-bounds-crash-expected.txt
new file mode 100644 (file)
index 0000000..0ebb2fe
--- /dev/null
@@ -0,0 +1,9 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 268x18
+          text run at (0,0) width 268: "PASS! This test passes if it does not crash."
+      RenderBlock {DIV} at (0,18) size 200x200
index 11cf49bbb795ee782eab490a4f52123845f860a6..e3ce65587f9ba149dc7d71fb943e25f76d8e01b5 100644 (file)
@@ -1,3 +1,34 @@
+2008-03-25  Beth Dakin  <bdakin@apple.com>
+
+        Reviewed by Oliver.
+
+        Fix for <rdar://problem/5811826> CSSValueList::item() does not 
+        range-check index
+
+        Check bounds before accessing the item to avoid a crash. 
+        itemWithoutBoundsCheck() is still inlined and not bounds-checked to 
+        avoid slowing down our internal callers of item().
+        * css/CSSValueList.cpp:
+        (WebCore::CSSValueList::item):
+        * css/CSSValueList.h:
+        (WebCore::CSSValueList::itemWithoutBoundsCheck):
+
+        Call itemWithoutBoundsCheck() to avoid slowing down these internal 
+        callers.
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::addFontFaceRule):
+        * css/CSSMutableStyleDeclaration.cpp:
+        (WebCore::CSSMutableStyleDeclaration::getLayeredShorthandValue):
+        * css/CSSStyleSelector.cpp:
+        (WebCore::applyCounterList):
+        (WebCore::CSSStyleSelector::applyProperty):
+        * css/MediaQueryEvaluator.cpp:
+        (WebCore::parseAspectRatio):
+        * svg/SVGFontFaceElement.cpp:
+        (WebCore::SVGFontFaceElement::rebuildFontFace):
+        * svg/graphics/SVGPaintServer.cpp:
+        (WebCore::dashArrayFromRenderingStyle):
+
 2008-03-25  Antti Koivisto  <antti@apple.com>
 
         Reviewed by Oliver.
index 3654a73b2784988cf520484023d992a69412b74b..61c6cee3e44af8031a4761d761984517deeda0c5 100644 (file)
@@ -145,7 +145,7 @@ void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule)
 
     for (i = 0; i < srcLength; i++) {
         // An item in the list either specifies a string (local font name) or a URL (remote font to download).
-        CSSFontFaceSrcValue* item = static_cast<CSSFontFaceSrcValue*>(srcList->item(i));
+        CSSFontFaceSrcValue* item = static_cast<CSSFontFaceSrcValue*>(srcList->itemWithoutBoundsCheck(i));
         CSSFontFaceSource* source = 0;
 
 #if ENABLE(SVG_FONTS)
@@ -203,7 +203,7 @@ void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule)
     // Hash under every single family name.
     int familyLength = familyList->length();
     for (i = 0; i < familyLength; i++) {
-        CSSPrimitiveValue* item = static_cast<CSSPrimitiveValue*>(familyList->item(i));
+        CSSPrimitiveValue* item = static_cast<CSSPrimitiveValue*>(familyList->itemWithoutBoundsCheck(i));
         String familyName;
         if (item->primitiveType() == CSSPrimitiveValue::CSS_STRING)
             familyName = static_cast<FontFamilyValue*>(item)->familyName();
@@ -260,7 +260,7 @@ void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule)
 
             unsigned numRanges = rangeList->length();
             for (unsigned i = 0; i < numRanges; i++) {
-                CSSUnicodeRangeValue* range = static_cast<CSSUnicodeRangeValue*>(rangeList->item(i));
+                CSSUnicodeRangeValue* range = static_cast<CSSUnicodeRangeValue*>(rangeList->itemWithoutBoundsCheck(i));
                 segmentedFontFace->overlayRange(range->from(), range->to(), fontFace);
             }
         } else
index 44dad706586bbc727d3576975d3cc251803fb4c4..1c1e1aae10ac02b1062e762d632ffde987adeb06 100644 (file)
@@ -204,7 +204,7 @@ String CSSMutableStyleDeclaration::getLayeredShorthandValue(const int* propertie
             RefPtr<CSSValue> value;
             if (values[j]) {
                 if (values[j]->isValueList())
-                    value = static_cast<CSSValueList*>(values[j].get())->item(i);
+                    value = static_cast<CSSValueList*>(values[j].get())->itemWithoutBoundsCheck(i);
                 else {
                     value = values[j];
                     
index a07852a3ff6a55bcaf32b02f288ec1cd2a5f94d5..4150ec5401e3229bf24ffad25aafd0ead0199749 100644 (file)
@@ -147,7 +147,7 @@ if (value->isValueList()) { \
             currChild = new LayerType(); \
             prevChild->setNext(currChild); \
         } \
-        map##Prop(currChild, valueList->item(i)); \
+        map##Prop(currChild, valueList->itemWithoutBoundsCheck(i)); \
         prevChild = currChild; \
         currChild = currChild->next(); \
     } \
@@ -2218,7 +2218,7 @@ static void applyCounterList(RenderStyle* style, CSSValueList* list, bool isRese
 
     int length = list ? list->length() : 0;
     for (int i = 0; i < length; ++i) {
-        Pair* pair = static_cast<CSSPrimitiveValue*>(list->item(i))->getPairValue();
+        Pair* pair = static_cast<CSSPrimitiveValue*>(list->itemWithoutBoundsCheck(i))->getPairValue();
         AtomicString identifier = static_cast<CSSPrimitiveValue*>(pair->first())->getStringValue();
         // FIXME: What about overflow?
         int value = static_cast<CSSPrimitiveValue*>(pair->second())->getIntValue();
@@ -2639,7 +2639,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
             int len = list->length();
             m_style->setCursor(CURSOR_AUTO);
             for (int i = 0; i < len; i++) {
-                CSSValue* item = list->item(i);
+                CSSValue* item = list->itemWithoutBoundsCheck(i);
                 if (!item->isPrimitiveValue())
                     continue;
                 primitiveValue = static_cast<CSSPrimitiveValue*>(item);
@@ -3358,7 +3358,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
 
         bool didSet = false;
         for (int i = 0; i < len; i++) {
-            CSSValue* item = list->item(i);
+            CSSValue* item = list->itemWithoutBoundsCheck(i);
             if (!item->isPrimitiveValue())
                 continue;
             
@@ -3445,7 +3445,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
         fontDescription.setGenericFamily(FontDescription::NoFamily);
 
         for (int i = 0; i < len; i++) {
-            CSSValue *item = list->item(i);
+            CSSValue *item = list->itemWithoutBoundsCheck(i);
             if (!item->isPrimitiveValue()) continue;
             CSSPrimitiveValue *val = static_cast<CSSPrimitiveValue*>(item);
             AtomicString face;
@@ -3514,7 +3514,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
             int len = list->length();
             for (int i = 0; i < len; i++)
             {
-                CSSValue *item = list->item(i);
+                CSSValue *item = list->itemWithoutBoundsCheck(i);
                 if (!item->isPrimitiveValue()) continue;
                 primitiveValue = static_cast<CSSPrimitiveValue*>(item);
                 switch (primitiveValue->getIdent()) {
@@ -3778,7 +3778,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
         CSSValueList* list = static_cast<CSSValueList*>(value);
         bool firstBinding = true;
         for (unsigned int i = 0; i < list->length(); i++) {
-            CSSValue *item = list->item(i);
+            CSSValue *item = list->itemWithoutBoundsCheck(i);
             CSSPrimitiveValue *val = static_cast<CSSPrimitiveValue*>(item);
             if (val->primitiveType() == CSSPrimitiveValue::CSS_URI) {
                 if (firstBinding) {
@@ -3947,7 +3947,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
         CSSValueList *list = static_cast<CSSValueList*>(value);
         int len = list->length();
         for (int i = 0; i < len; i++) {
-            ShadowValue* item = static_cast<ShadowValue*>(list->item(i));
+            ShadowValue* item = static_cast<ShadowValue*>(list->itemWithoutBoundsCheck(i));
             int x = item->x->computeLengthInt(m_style, zoomFactor);
             int y = item->y->computeLengthInt(m_style, zoomFactor);
             int blur = item->blur ? item->blur->computeLengthInt(m_style, zoomFactor) : 0;
@@ -4354,10 +4354,10 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
             CSSValueList* list = static_cast<CSSValueList*>(value);
             unsigned size = list->length();
             for (unsigned i = 0; i < size; i++) {
-                CSSTransformValue* val = static_cast<CSSTransformValue*>(list->item(i));
+                CSSTransformValue* val = static_cast<CSSTransformValue*>(list->itemWithoutBoundsCheck(i));
                 CSSValueList* values = val->values();
                 
-                CSSPrimitiveValue* firstValue = static_cast<CSSPrimitiveValue*>(values->item(0));
+                CSSPrimitiveValue* firstValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(0));
                  
                 switch (val->type()) {
                     case CSSTransformValue::ScaleTransformOperation:
@@ -4371,7 +4371,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
                             sx = firstValue->getDoubleValue();
                             if (val->type() == CSSTransformValue::ScaleTransformOperation) {
                                 if (values->length() > 1) {
-                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->item(1));
+                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(1));
                                     sy = secondValue->getDoubleValue();
                                 } else 
                                     sy = sx;
@@ -4394,7 +4394,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
                             tx = convertToLength(firstValue, m_style, &ok);
                             if (val->type() == CSSTransformValue::TranslateTransformOperation) {
                                 if (values->length() > 1) {
-                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->item(1));
+                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(1));
                                     ty = convertToLength(secondValue, m_style, &ok);
                                 } else
                                     ty = tx;
@@ -4431,7 +4431,7 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
                             angleX = angle;
                             if (val->type() == CSSTransformValue::SkewTransformOperation) {
                                 if (values->length() > 1) {
-                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->item(1));
+                                    CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(1));
                                     angleY = secondValue->getDoubleValue();
                                     if (secondValue->primitiveType() == CSSPrimitiveValue::CSS_RAD)
                                         angleY = rad2deg(angle);
@@ -4447,11 +4447,11 @@ void CSSStyleSelector::applyProperty(int id, CSSValue *value)
                         break;
                     }
                     case CSSTransformValue::MatrixTransformOperation: {
-                        CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->item(1));
-                        CSSPrimitiveValue* thirdValue = static_cast<CSSPrimitiveValue*>(values->item(2));
-                        CSSPrimitiveValue* fourthValue = static_cast<CSSPrimitiveValue*>(values->item(3));
-                        CSSPrimitiveValue* fifthValue = static_cast<CSSPrimitiveValue*>(values->item(4));
-                        CSSPrimitiveValue* sixthValue = static_cast<CSSPrimitiveValue*>(values->item(5));
+                        CSSPrimitiveValue* secondValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(1));
+                        CSSPrimitiveValue* thirdValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(2));
+                        CSSPrimitiveValue* fourthValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(3));
+                        CSSPrimitiveValue* fifthValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(4));
+                        CSSPrimitiveValue* sixthValue = static_cast<CSSPrimitiveValue*>(values->itemWithoutBoundsCheck(5));
                         MatrixTransformOperation* matrix = new MatrixTransformOperation(firstValue->getDoubleValue(),
                                                                                         secondValue->getDoubleValue(),
                                                                                         thirdValue->getDoubleValue(),
index c845ab99957cafc1c4576d7d41c329426424ffc1..8df6a9f9e0e4a3b302e1db8bc27ab4fbb6f90fc4 100644 (file)
@@ -35,6 +35,13 @@ CSSValueList::~CSSValueList()
 {
 }
 
+CSSValue* CSSValueList::item(unsigned index)
+{
+    if (index >= m_values.size())
+        return 0;
+    return m_values[index].get();
+}
+
 unsigned short CSSValueList::cssValueType() const
 {
     return CSS_VALUE_LIST;
index 405df52e89816e318da4dab33d0581d0901b94c5..21b0855a05d9e9abedd8f431c00c0af457bdfe8b 100644 (file)
@@ -35,7 +35,8 @@ public:
     virtual ~CSSValueList();
 
     unsigned length() const { return m_values.size(); }
-    CSSValue* item (unsigned index) { return m_values[index].get(); }
+    CSSValue* item(unsigned);
+    CSSValue* itemWithoutBoundsCheck(unsigned index) { return m_values[index].get(); }
 
     virtual bool isValueList() { return true; }
 
index d5ea65e225b0dee4f6eca78cda34bc7753cd7e0b..650b88fbf0929686d1230b33390772751486a477 100644 (file)
@@ -165,9 +165,9 @@ static bool parseAspectRatio(CSSValue* value, int& h, int& v)
     if (value->isValueList()){
         CSSValueList* valueList = static_cast<CSSValueList*>(value);
         if (valueList->length() == 3) {
-            CSSValue* i0 = valueList->item(0);
-            CSSValue* i1 = valueList->item(1);
-            CSSValue* i2 = valueList->item(2);
+            CSSValue* i0 = valueList->itemWithoutBoundsCheck(0);
+            CSSValue* i1 = valueList->itemWithoutBoundsCheck(1);
+            CSSValue* i2 = valueList->itemWithoutBoundsCheck(2);
             if (i0->isPrimitiveValue() && static_cast<CSSPrimitiveValue*>(i0)->primitiveType() == CSSPrimitiveValue::CSS_NUMBER
                 && i1->isPrimitiveValue() && static_cast<CSSPrimitiveValue*>(i1)->primitiveType() == CSSPrimitiveValue::CSS_STRING
                 && i2->isPrimitiveValue() && static_cast<CSSPrimitiveValue*>(i2)->primitiveType() == CSSPrimitiveValue::CSS_NUMBER) {
index d31c52459b61d1623f4407861a265db7ffd81273..857ed66beeca86e0f7e22ec7b0496bfdd9971e6c 100644 (file)
@@ -339,7 +339,7 @@ void SVGFontFaceElement::rebuildFontFace()
 
         unsigned srcLength = srcList ? srcList->length() : 0;
         for (unsigned i = 0; i < srcLength; i++) {
-            if (CSSFontFaceSrcValue* item = static_cast<CSSFontFaceSrcValue*>(srcList->item(i)))
+            if (CSSFontFaceSrcValue* item = static_cast<CSSFontFaceSrcValue*>(srcList->itemWithoutBoundsCheck(i)))
                 item->setSVGFontFaceElement(this);
         }
     }
index 9a596833a1d7e6383c3421d66b62317667c5c89d..3dcfbbf289387f184af98a6a3b974013c0fc914f 100644 (file)
@@ -153,7 +153,7 @@ DashArray dashArrayFromRenderingStyle(const RenderStyle* style)
         CSSPrimitiveValue* dash = 0;
         unsigned long len = dashes->length();
         for (unsigned long i = 0; i < len; i++) {
-            dash = static_cast<CSSPrimitiveValue*>(dashes->item(i));
+            dash = static_cast<CSSPrimitiveValue*>(dashes->itemWithoutBoundsCheck(i));
             if (!dash)
                 continue;