commit a54f0f23fb610f696b79ae929c0ccfa32e2e6721
author fmalita@chromium.org <fmalita@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Wed Feb 13 17:49:47 2013 +0000
committer fmalita@chromium.org <fmalita@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Wed Feb 13 17:49:47 2013 +0000
tree bd892af9ae664270167db03c345a347db9f95854
parent 61a779056425eb49828995a30835fdbcaa891ee9

[SVG] OOB access in SVGListProperty::replaceItemValues()
https://bugs.webkit.org/show_bug.cgi?id=109293

Source/WebCore:

Replacing a list property item with itself should be a no-op. This patch updates the related
APIs and logic to detect the self-replace case and prevent removal of the item from the list.

To avoid scanning the list multiple times, removeItemFromList() is updated to operate on
indices and a findItem() method is added to resolve an item to an index.

Reviewed by Dirk Schulze.

No new tests: updated existing tests cover the change.

* svg/properties/SVGAnimatedListPropertyTearOff.h:
(WebCore::SVGAnimatedListPropertyTearOff::findItem):
(SVGAnimatedListPropertyTearOff):
(WebCore::SVGAnimatedListPropertyTearOff::removeItemFromList):
* svg/properties/SVGAnimatedPathSegListPropertyTearOff.h:
(WebCore::SVGAnimatedPathSegListPropertyTearOff::findItem):
(SVGAnimatedPathSegListPropertyTearOff):
(WebCore::SVGAnimatedPathSegListPropertyTearOff::removeItemFromList):
Add a findItem() delegating method, and update removeItemFromList() to use the new
index-based API.

* svg/properties/SVGListProperty.h:
(WebCore::SVGListProperty::insertItemBeforeValues):
(WebCore::SVGListProperty::insertItemBeforeValuesAndWrappers):
(WebCore::SVGListProperty::replaceItemValues):
(WebCore::SVGListProperty::replaceItemValuesAndWrappers):
(SVGListProperty):
Updated to handle the no-op case for insertItemBefore() & replaceItem().

* svg/properties/SVGListPropertyTearOff.h:
(WebCore::SVGListPropertyTearOff::findItem):
(WebCore::SVGListPropertyTearOff::removeItemFromList):
Index-based API updates.

(WebCore::SVGListPropertyTearOff::processIncomingListItemValue):
(WebCore::SVGListPropertyTearOff::processIncomingListItemWrapper):
* svg/properties/SVGPathSegListPropertyTearOff.cpp:
(WebCore::SVGPathSegListPropertyTearOff::processIncomingListItemValue):
Detect the self-replace case and return without removing the item from the list.

* svg/properties/SVGPathSegListPropertyTearOff.h:
(WebCore::SVGPathSegListPropertyTearOff::findItem):
(WebCore::SVGPathSegListPropertyTearOff::removeItemFromList):
(SVGPathSegListPropertyTearOff):
(WebCore::SVGPathSegListPropertyTearOff::processIncomingListItemWrapper):
* svg/properties/SVGStaticListPropertyTearOff.h:
(WebCore::SVGStaticListPropertyTearOff::processIncomingListItemValue):
(WebCore::SVGStaticListPropertyTearOff::processIncomingListItemWrapper):
Index-based API updates.

LayoutTests:

Updated tests to cover the crash and new behavior.

Reviewed by Dirk Schulze.

* svg/dom/SVGLengthList-basics-expected.txt:
* svg/dom/SVGLengthList-basics.xhtml:
* svg/dom/SVGNumberList-basics-expected.txt:
* svg/dom/SVGNumberList-basics.xhtml:
* svg/dom/SVGPointList-basics-expected.txt:
* svg/dom/SVGPointList-basics.xhtml:
* svg/dom/SVGTransformList-basics-expected.txt:
* svg/dom/SVGTransformList-basics.xhtml:


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

LayoutTests/svg/dom/SVGPointList-basics.xhtml

diff --git a/LayoutTests/svg/dom/SVGPointList-basics.xhtml b/LayoutTests/svg/dom/SVGPointList-basics.xhtml
index d078e64..b13a215 100644
--- a/LayoutTests/svg/dom/SVGPointList-basics.xhtml
+++ b/LayoutTests/svg/dom/SVGPointList-basics.xhtml
@@ -14,6 +14,9 @@
 <![CDATA[
     description("This is a test of the simple SVGPointList API parts.");
 
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
     // Extend String prototype, to offer a function, that formats the points attribute in the same way across browsers
     String.prototype.formatPointsAttribute = function() {
         return this.replace(/,/g, " "); // Remove Firefox commas
@@ -129,26 +132,32 @@
     shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 100 0 100 100 0 100");
 
     shouldBeEqualToString("dumpPoint(poly1.points.replaceItem(poly1.points.getItem(0), 0))", "x=0 y=0");
-    shouldBe("poly1.points.numberOfItems", "3");
+    shouldBe("poly1.points.numberOfItems", "4");
     shouldBeEqualToString("dumpPoint(poly1.points.getItem(0))", "x=0 y=0");
-    shouldBeEqualToString("dumpPoint(poly1.points.getItem(1))", "x=100 y=100");
-    shouldBeEqualToString("dumpPoint(poly1.points.getItem(2))", "x=0 y=100");
-    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 100 100 0 100");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(1))", "x=100 y=0");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(2))", "x=100 y=100");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(3))", "x=0 y=100");
+    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 100 0 100 100 0 100");
 
     shouldBeEqualToString("dumpPoint(poly1.points.replaceItem(poly1.points.getItem(0), 'aString'))", "x=0 y=0");
-    shouldBe("poly1.points.numberOfItems", "2");
+    shouldBe("poly1.points.numberOfItems", "4");
     shouldBeEqualToString("dumpPoint(poly1.points.getItem(0))", "x=0 y=0");
-    shouldBeEqualToString("dumpPoint(poly1.points.getItem(1))", "x=0 y=100");
-    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 0 100");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(1))", "x=100 y=0");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(2))", "x=100 y=100");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(3))", "x=0 y=100");
+    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 100 0 100 100 0 100");
 
     shouldBeEqualToString("dumpPoint(poly1.points.replaceItem(poly1.points.getItem(0), poly1))", "x=0 y=0");
-    shouldBe("poly1.points.numberOfItems", "1");
+    shouldBe("poly1.points.numberOfItems", "4");
     shouldBeEqualToString("dumpPoint(poly1.points.getItem(0))", "x=0 y=0");
-    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(1))", "x=100 y=0");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(2))", "x=100 y=100");
+    shouldBeEqualToString("dumpPoint(poly1.points.getItem(3))", "x=0 y=100");
+    shouldBeEqualToString("poly1.getAttribute('points').formatPointsAttribute()", "0 0 100 0 100 100 0 100");
 
-    shouldThrow("poly1.points.replaceItem(poly1.points.getItem(0), null)");
-    shouldBe("poly1.points.numberOfItems", "0");
-    shouldBeEqualToString("poly1.getAttribute('points')", "");
+    shouldBeEqualToString("dumpPoint(poly1.points.replaceItem(poly1.points.getItem(0), null))", "x=0 y=0");
+    shouldBe("poly1.points.numberOfItems", "4");
+    shouldBeEqualToString("poly1.getAttribute('points')", "0 0 100 0 100 100 0 100");
 
     debug("");
     debug("Reset points attribute to 0 0 100 0 100 100 0 100");