2010-09-06 Adam Barth <abarth@webkit.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Sep 2010 04:06:16 +0000 (04:06 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 7 Sep 2010 04:06:16 +0000 (04:06 +0000)
        Reviewed by Sam Weinig.

        OOB read with svg polyline
        https://bugs.webkit.org/show_bug.cgi?id=45279

        In principle, attributeChanged can do anything.  If we supported more
        DOM mutation events, it could even run JavaScript.  That means we need
        to be prepared for the attribute map to change when running
        attributeChanged.  This patch makes this loop resilient to the
        attribute map changing by storing the list of changed attributes on the
        stack.

        Test: fast/parser/changing-attrbutes-crash.html

        * dom/Element.cpp:
        (WebCore::Element::setAttributeMap):
2010-09-06  Adam Barth  <abarth@webkit.org>

        Reviewed by Sam Weinig.

        OOB read with svg polyline
        https://bugs.webkit.org/show_bug.cgi?id=45279

        Test what happens when SVG changes the attribute map out from under us.

        * fast/parser/changing-attrbutes-crash-expected.txt: Added.
        * fast/parser/changing-attrbutes-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/parser/changing-attrbutes-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/parser/changing-attrbutes-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/Element.cpp

index b38fbaf..7f7e728 100644 (file)
@@ -1,3 +1,15 @@
+2010-09-06  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Sam Weinig.
+
+        OOB read with svg polyline
+        https://bugs.webkit.org/show_bug.cgi?id=45279
+
+        Test what happens when SVG changes the attribute map out from under us.
+
+        * fast/parser/changing-attrbutes-crash-expected.txt: Added.
+        * fast/parser/changing-attrbutes-crash.html: Added.
+
 2010-09-06  Kent Tamura  <tkent@chromium.org>
 
         Reviewed by Dimitri Glazkov.
diff --git a/LayoutTests/fast/parser/changing-attrbutes-crash-expected.txt b/LayoutTests/fast/parser/changing-attrbutes-crash-expected.txt
new file mode 100644 (file)
index 0000000..83b7e07
--- /dev/null
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 0: Error: Problem parsing points="foo"
+This test passes if it doesn't crash.
diff --git a/LayoutTests/fast/parser/changing-attrbutes-crash.html b/LayoutTests/fast/parser/changing-attrbutes-crash.html
new file mode 100644 (file)
index 0000000..cd4d578
--- /dev/null
@@ -0,0 +1,6 @@
+<svg><polygon class="bar" points="foo"></svg>
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<p>This test passes if it doesn't crash.</p>
index a6a6450..a685375 100644 (file)
@@ -1,3 +1,22 @@
+2010-09-06  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Sam Weinig.
+
+        OOB read with svg polyline
+        https://bugs.webkit.org/show_bug.cgi?id=45279
+
+        In principle, attributeChanged can do anything.  If we supported more
+        DOM mutation events, it could even run JavaScript.  That means we need
+        to be prepared for the attribute map to change when running
+        attributeChanged.  This patch makes this loop resilient to the
+        attribute map changing by storing the list of changed attributes on the
+        stack.
+
+        Test: fast/parser/changing-attrbutes-crash.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::setAttributeMap):
+
 2010-09-06  Oliver Hunt  <oliver@apple.com>
 
         Windows build fix
index 7c888bc..69bd160 100644 (file)
@@ -695,9 +695,12 @@ void Element::setAttributeMap(PassRefPtr<NamedNodeMap> list, FragmentScriptingPe
                 i++;
             }
         }
-        unsigned len = m_attributeMap->length();
-        for (unsigned i = 0; i < len; i++)
-            attributeChanged(m_attributeMap->m_attributes[i].get());
+        // Store the set of attributes that changed on the stack in case
+        // attributeChanged mutates m_attributeMap.
+        Vector<RefPtr<Attribute> > attributes;
+        m_attributeMap->copyAttributesToVector(attributes);
+        for (Vector<RefPtr<Attribute> >::iterator iter = attributes.begin(); iter != attributes.end(); ++iter)
+            attributeChanged(iter->get());
         // FIXME: What about attributes that were in the old map that are not in the new map?
     }
 }