- Revision
- 109299
- Author
- [email protected]
- Date
- 2012-02-29 19:54:24 -0800 (Wed, 29 Feb 2012)
Log Message
SVG <use> element allows invalid contents
https://bugs.webkit.org/show_bug.cgi?id=77764
Patch by Stephen Chenney <[email protected]> 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (109298 => 109299)
--- trunk/LayoutTests/ChangeLog 2012-03-01 03:38:02 UTC (rev 109298)
+++ trunk/LayoutTests/ChangeLog 2012-03-01 03:54:24 UTC (rev 109299)
@@ -1,3 +1,23 @@
+2012-02-29 Stephen Chenney <[email protected]>
+
+ 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 <[email protected]>
Continue the search for playable mime types among <source> children of <video> even when using data: URLs
Added: trunk/LayoutTests/svg/custom/bug78807-expected.txt (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug78807-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/bug78807-expected.txt 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,6 @@
+This page contains the following errors:
+
+error on line 11 at column 19: Extra content at the end of the document
+Below is a rendering of the page up to the first error.
+
+
Added: trunk/LayoutTests/svg/custom/bug78807.svg (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug78807.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/bug78807.svg 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,11 @@
+<svg style="content:counter(counter_0);" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <!-- This test passes if there is no crash. -->
+ <script>
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+ </script>
+ <g id="g">
+ <style>
+ </style>
+ </g>
+ <use xlink:href=""
Added: trunk/LayoutTests/svg/custom/bug78838-expected.txt (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug78838-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/bug78838-expected.txt 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,2 @@
+
+PASSED
Added: trunk/LayoutTests/svg/custom/bug78838.html (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug78838.html (rev 0)
+++ trunk/LayoutTests/svg/custom/bug78838.html 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,19 @@
+<!-- This test passes if there is no crash. -->
+<script>
+function go() {
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ q = document.getElementById('root').contentDocument;
+ q.firstChild.setAttribute('style', 'content:counter(item)');
+
+ document.designMode = 'on';
+ q.execCommand('selectAll');
+ q.execCommand('insertImage');
+ q.getElementById('x').appendChild( q.firstChild.cloneNode(1) );
+ q.execCommand('undo');
+}
+</script>
+<object data="" id="root" _onload_="go()"/></object>
+<p>PASSED</p>
Added: trunk/LayoutTests/svg/custom/bug79798-expected.txt (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug79798-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/bug79798-expected.txt 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1 @@
+
Added: trunk/LayoutTests/svg/custom/bug79798.html (0 => 109299)
--- trunk/LayoutTests/svg/custom/bug79798.html (rev 0)
+++ trunk/LayoutTests/svg/custom/bug79798.html 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,24 @@
+<!-- This test passes if there is no crash. -->
+<!-- It was created by the fuzzer and has been reduced. -->
+<script>
+ function go() {
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ q = document.getElementById('root').contentDocument;
+
+ r = document.createRange();
+ s = window.getSelection();
+
+ r.selectNodeContents( q.getElementById('g') );
+ s.addRange( r );
+
+ document.designMode = 'on';
+ q.execCommand('formatBlock', 1, 'h1');
+ q.execCommand('formatBlock', 1, 'h1');
+ q.execCommand('formatBlock', 1, 'h1');
+ document.open();
+ }
+</script>
+<object data="" id="root" _onload_="go()"/</object>
Added: trunk/LayoutTests/svg/custom/resources/bug78838.svg (0 => 109299)
--- trunk/LayoutTests/svg/custom/resources/bug78838.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/resources/bug78838.svg 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,6 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <text id="x">a</text>
+ <use xlink:href=""
+ <text>a</text>
+ <style></style>
+</svg>
Added: trunk/LayoutTests/svg/custom/resources/bug79798.svg (0 => 109299)
--- trunk/LayoutTests/svg/custom/resources/bug79798.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/resources/bug79798.svg 2012-03-01 03:54:24 UTC (rev 109299)
@@ -0,0 +1,15 @@
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <g id="x">
+ <style></style>
+ </g>
+
+ <g>
+ <g>
+ <text>b</text>
+ <use xlink:href=""
+ <text>c</text>
+ <text>a</text>
+ <glyph id="g"/>
+ </g>
+ </g>
+</svg>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (109298 => 109299)
--- trunk/Source/WebCore/ChangeLog 2012-03-01 03:38:02 UTC (rev 109298)
+++ trunk/Source/WebCore/ChangeLog 2012-03-01 03:54:24 UTC (rev 109299)
@@ -1,3 +1,24 @@
+2012-02-29 Stephen Chenney <[email protected]>
+
+ 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 <[email protected]>
Continue the search for playable mime types among <source> children of <video> even when using data: URLs
Modified: trunk/Source/WebCore/svg/SVGUseElement.cpp (109298 => 109299)
--- trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-03-01 03:38:02 UTC (rev 109298)
+++ trunk/Source/WebCore/svg/SVGUseElement.cpp 2012-03-01 03:54:24 UTC (rev 109299)
@@ -275,14 +275,46 @@
}
#endif
-static bool isDisallowedElement(Node* element)
+static bool isDisallowedElement(Node* node)
{
- // <foreignObject> should never be contained in a <use> tree. Too dangerous side effects possible.
- if (element->hasTagName(SVGNames::foreignObjectTag))
+ // 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());
}
static bool subtreeContainsDisallowedElement(Node* start)
