Changeset 109299 in webkit

Timestamp:

Feb 29, 2012, 7:54:24 PM (15 years ago)

Author:

commit-queue@webkit.org

Message:

SVG <use> element allows invalid contents
​https://bugs.webkit.org/show_bug.cgi?id=77764

Patch by Stephen Chenney <​schenney@chromium.org> on 2012-02-29
Reviewed by Nikolas Zimmermann.

Source/WebCore:

Modify the isDisallowedElement method to disallow all of the
disallowed elements, instead of just a few. It is now a whitelist
implementation.

This also fixes bugs 78807, 78838 and 79798 related to memory
corruption issues.

Tests: svg/custom/bug78807.svg

svg/custom/bug78838.html
svg/custom/bug79798.html

• svg/SVGUseElement.cpp:

(WebCore::isDisallowedElement):

LayoutTests:

These test all use invalid elements in the <use> and crash in the absence
of this patch. Existing tests cover the valid elements in <use>, so no
additional tests for those.

• svg/custom/bug78807-expected.txt: Added.
• svg/custom/bug78807.svg: Added.
• svg/custom/bug78838-expected.txt: Added.
• svg/custom/bug78838.html: Added.
• svg/custom/bug79798-expected.txt: Added.
• svg/custom/bug79798.html: Added.
• svg/custom/resources/bug78838.svg: Added.
• svg/custom/resources/bug79798.svg: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/svg/custom/bug78807-expected.txt (added)
• LayoutTests/svg/custom/bug78807.svg (added)
• LayoutTests/svg/custom/bug78838-expected.txt (added)
• LayoutTests/svg/custom/bug78838.html (added)
• LayoutTests/svg/custom/bug79798-expected.txt (added)
• LayoutTests/svg/custom/bug79798.html (added)
• LayoutTests/svg/custom/resources/bug78838.svg (added)
• LayoutTests/svg/custom/resources/bug79798.svg (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/svg/SVGUseElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-02-29  Stephen Chenney  <schenney@chromium.org>
+
+        SVG <use> element allows invalid contents
+        https://bugs.webkit.org/show_bug.cgi?id=77764
+
+        Reviewed by Nikolas Zimmermann.
+
+        These test all use invalid elements in the <use> and crash in the absence
+        of this patch. Existing tests cover the valid elements in <use>, so no
+        additional tests for those.
+
+        * svg/custom/bug78807-expected.txt: Added.
+        * svg/custom/bug78807.svg: Added.
+        * svg/custom/bug78838-expected.txt: Added.
+        * svg/custom/bug78838.html: Added.
+        * svg/custom/bug79798-expected.txt: Added.
+        * svg/custom/bug79798.html: Added.
+        * svg/custom/resources/bug78838.svg: Added.
+        * svg/custom/resources/bug79798.svg: Added.
+
 2012-02-29  Ami Fischman  <fischman@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-02-29  Stephen Chenney  <schenney@chromium.org>
+
+        SVG <use> element allows invalid contents
+        https://bugs.webkit.org/show_bug.cgi?id=77764
+
+        Reviewed by Nikolas Zimmermann.
+
+        Modify the isDisallowedElement method to disallow all of the
+        disallowed elements, instead of just a few. It is now a whitelist
+        implementation.
+
+        This also fixes bugs 78807, 78838 and 79798 related to memory
+        corruption issues.
+
+        Tests: svg/custom/bug78807.svg
+               svg/custom/bug78838.html
+               svg/custom/bug79798.html
+
+        * svg/SVGUseElement.cpp:
+        (WebCore::isDisallowedElement):
+
 2012-02-29  Ami Fischman  <fischman@chromium.org>
 

trunk/Source/WebCore/svg/SVGUseElement.cpp

@@ -276 +276 @@
 #endif
 
-static bool isDisallowedElement(Node* element)
-{
-    // <foreignObject> should never be contained in a <use> tree. Too dangerous side effects possible.
-    if (element->hasTagName(SVGNames::foreignObjectTag))
+static bool isDisallowedElement(Node* node)
+{
+    // Spec: "Any 'svg', 'symbol', 'g', graphics element or other 'use' is potentially a template object that can be re-used
+    // (i.e., "instanced") in the SVG document via a 'use' element."
+    // "Graphics Element" is defined as 'circle', 'ellipse', 'image', 'line', 'path', 'polygon', 'polyline', 'rect', 'text'
+    // Excluded are anything that is used by reference or that only make sense to appear once in a document.
+    // We must also allow the shadow roots of other use elements.
+    if (node->isShadowRoot())
+        return false;
+
+    if (!node->isSVGElement())
         return true;
-    if (SVGSMILElement::isSMILElement(element))
-        return true;
-    return false;
+
+    Element* element = static_cast<Element*>(node);
+
+    DEFINE_STATIC_LOCAL(HashSet<String>, allowedElementTags, ());
+    if (allowedElementTags.isEmpty()) {
+        allowedElementTags.add(SVGNames::aTag.toString());
+        allowedElementTags.add(SVGNames::circleTag.toString());
+        allowedElementTags.add(SVGNames::descTag.toString());
+        allowedElementTags.add(SVGNames::ellipseTag.toString());
+        allowedElementTags.add(SVGNames::gTag.toString());
+        allowedElementTags.add(SVGNames::imageTag.toString());
+        allowedElementTags.add(SVGNames::lineTag.toString());
+        allowedElementTags.add(SVGNames::metadataTag.toString());
+        allowedElementTags.add(SVGNames::pathTag.toString());
+        allowedElementTags.add(SVGNames::polygonTag.toString());
+        allowedElementTags.add(SVGNames::polylineTag.toString());
+        allowedElementTags.add(SVGNames::rectTag.toString());
+        allowedElementTags.add(SVGNames::svgTag.toString());
+        allowedElementTags.add(SVGNames::switchTag.toString());
+        allowedElementTags.add(SVGNames::symbolTag.toString());
+        allowedElementTags.add(SVGNames::textTag.toString());
+        allowedElementTags.add(SVGNames::textPathTag.toString());
+        allowedElementTags.add(SVGNames::titleTag.toString());
+        allowedElementTags.add(SVGNames::trefTag.toString());
+        allowedElementTags.add(SVGNames::tspanTag.toString());
+        allowedElementTags.add(SVGNames::useTag.toString());
+    }
+    return !allowedElementTags.contains(element->tagName());
 }