Exploitable crash happens when an SVG contains an indirect resource inheritance cycle
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Oct 2015 17:12:43 +0000 (17:12 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Oct 2015 17:12:43 +0000 (17:12 +0000)
commit3f730dbf43d4d3effb09705be6c0972ec6ce8b8e
treeb21f0b741718687a1a10d4fd17d3b8de3447bba6
parent948d8aec0b691a1f9f2ac2eb745432147d6ea9b5
Exploitable crash happens when an SVG contains an indirect resource inheritance cycle
https://bugs.webkit.org/show_bug.cgi?id=150203

Reviewed by Brent Fulgham.

Source/WebCore:

Detecting cycles in SVG resource references happens in two places.
1. In SVGResourcesCycleSolver::resolveCycles() which it is called from
   SVGResourcesCache::addResourcesFromRenderer(). When a cycle is deleted,
   SVGResourcesCycleSolver::breakCycle() is called to break the link. In
   the case of a cyclic resource inheritance, SVGResources::resetLinkedResource()
   is called to break this cycle.
2. SVGPatternElement::collectPatternAttributes() which is called from
   RenderSVGResourcePattern::buildPattern(). The purpose is to resolve
   the pattern attributes and to build a tile image which can be used to
   fill the SVG element renderer. Detecting the cyclic resource reference
   in this function is not sufficient and can detect simple cycles like
    <pattern id="a" xlink:href="#b"/>
    <pattern id="b" xlink:href="#a"/>.
   But it does not detect cycles like:
    <pattern id="a">
        <rect fill="url(#b)"/>
    </pattern>
    <pattern id="b" xlink:href="#a"/>.

The fix is to get rid of SVGPatternElement::collectPatternAttributes() which
uses SVGURIReference::targetElementFromIRIString() to navigates through the
referenced resource elements and tries to detect cycles. Instead we can
implement RenderSVGResourcePattern::collectPatternAttributes() which calls
SVGResourcesCache::cachedResourcesForRenderer() to get the SVGResources
of the pattern. Then we use SVGResources::linkedResource() to navigate the
resource inheritance tree. The cached SVGResources is guaranteed to be free
of cycles.

Tests: svg/custom/pattern-content-inheritance-cycle.svg

* rendering/svg/RenderSVGResourcePattern.cpp:
(WebCore::RenderSVGResourcePattern::collectPatternAttributes):
Collect the pattern attributes through the cachedResourcesForRenderer().

(WebCore::RenderSVGResourcePattern::buildPattern):
Direct the call to the renderer function.

* rendering/svg/RenderSVGResourcePattern.h:

* rendering/svg/RenderSVGRoot.cpp:
(WebCore::RenderSVGRoot::layout):
RenderSVGRoot needs to call SVGResourcesCache::clientStyleChanged() for all
the invalidated resources. If an attribute of an SVG resource was updated
dynamically, the cached SVGResources associated with the renderer of this
resource was stale.

* rendering/svg/SVGRenderTreeAsText.cpp:
(WebCore::writeSVGResourceContainer):
Direct the call to the renderer function.

* svg/SVGPatternElement.cpp:
(WebCore::SVGPatternElement::collectPatternAttributes):
(WebCore::setPatternAttributes): Deleted.
collectPatternAttributes() is a replacement of setPatternAttributes().

LayoutTests:

Ensure that we do not crash when an SVG has an indirect cyclic resource
inheritance. Make sure the cyclic resource was just ignored as if it did
not exist.

* svg/custom/pattern-content-inheritance-cycle-expected.svg: Added.
* svg/custom/pattern-content-inheritance-cycle.svg: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@191731 268f45cc-cd09-0410-ab3c-d52691b4dbfc
LayoutTests/ChangeLog
LayoutTests/svg/custom/pattern-content-inheritance-cycle-expected.svg [new file with mode: 0644]
LayoutTests/svg/custom/pattern-content-inheritance-cycle.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp
Source/WebCore/rendering/svg/RenderSVGResourcePattern.h
Source/WebCore/rendering/svg/RenderSVGRoot.cpp
Source/WebCore/rendering/svg/SVGRenderTreeAsText.cpp
Source/WebCore/svg/SVGPatternElement.cpp