Bug 119623

Summary: ASSERTION FAILED: stroke->opacity != other->stroke->opacity in WebCore::SVGRenderStyle::diff
Product: WebKit Reporter: Renata Hodovan <rhodovan.u-szeged>
Component: Layout and RenderingAssignee: Rob Buis <rwlbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kling, kondapallykalyan, krit, rwlbuis, sam, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 116980    
Attachments:
Description Flags
Test case (extension needs to be .html)
none
Patch krit: review+

Description Renata Hodovan 2013-08-09 05:34:48 PDT
The following test crashes:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <text>
     <tspan stroke="#666">
        <a xlink:href="#foo">
           <animate attributename="stroke"></animate>
     </tspan>
  </text>
</svg>


The backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff56f53e4 in WTFCrash () at /home/reni/Data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:342
342	    *(int *)(uintptr_t)0xbbadbeef = 0;
(gdb) bt
#0  0x00007ffff56f53e4 in WTFCrash () at /home/reni/Data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:342
#1  0x00007ffff4ba3754 in WebCore::SVGRenderStyle::diff (this=0x8bf0b0, other=0x727520)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/rendering/style/SVGRenderStyle.cpp:190
#2  0x00007ffff4a034a9 in WebCore::RenderStyle::diff (this=0x8bf040, other=0x72be10, changedContextSensitiveProperties=@0x7fffffffc554: 0)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/rendering/style/RenderStyle.cpp:759
#3  0x00007ffff49740eb in WebCore::RenderObject::setStyle (this=0x8baa28, style=...)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/rendering/RenderObject.cpp:1838

#4  0x00007ffff4973ad3 in WebCore::RenderObject::setAnimatableStyle (this=0x8baa28, style=...)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/rendering/RenderObject.cpp:1730
#5  0x00007ffff4a29efa in WebCore::Style::resolveLocal (current=0x8be9d0, inheritedChange=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:156
#6  0x00007ffff4a2a3aa in WebCore::Style::resolveTree (current=0x8be9d0, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:234
#7  0x00007ffff4a2a582 in WebCore::Style::resolveTree (current=0x8bad00, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:268
#8  0x00007ffff4a2a582 in WebCore::Style::resolveTree (current=0x89aaa0, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:268
#9  0x00007ffff4a2a582 in WebCore::Style::resolveTree (current=0x744d30, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:268
#10 0x00007ffff4a2a582 in WebCore::Style::resolveTree (current=0x87f2d0, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:268
#11 0x00007ffff4a2a582 in WebCore::Style::resolveTree (current=0x7754e0, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/style/StyleResolveTree.cpp:268
#12 0x00007ffff41b097d in WebCore::Document::recalcStyle (this=0x87c980, change=WebCore::Style::NoChange)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:1804
#13 0x00007ffff41b0cd5 in WebCore::Document::updateStyleIfNeeded (this=0x87c980) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:1856
#14 0x00007ffff401440c in WebCore::ComputedStyleExtractor::propertyValue (this=0x7fffffffcdb0, propertyID=WebCore::CSSPropertyStroke, 
    updateLayout=WebCore::UpdateLayout) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1681

#15 0x00007ffff4c5e3a8 in WebCore::SVGAnimationElement::computeCSSPropertyValue (this=0x8b7bc0, element=0x8be9d0, id=WebCore::CSSPropertyStroke, 
    valueString=...) at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/svg/SVGAnimationElement.cpp:642
#16 0x00007ffff4c570f3 in WebCore::SVGAnimateElement::resetAnimatedType (this=0x8b7bc0)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/svg/SVGAnimateElement.cpp:233
#17 0x00007ffff4c09a48 in WebCore::SVGSMILElement::progress (this=0x8b7bc0, elapsed=..., resultElement=0x8b7bc0, seekToTime=false)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/svg/animation/SVGSMILElement.cpp:1107
#18 0x00007ffff4bffc4d in WebCore::SMILTimeContainer::updateAnimations (this=0x893cb0, elapsed=..., seekToTime=false)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/svg/animation/SMILTimeContainer.cpp:293
#19 0x00007ffff4bff717 in WebCore::SMILTimeContainer::timerFired (this=0x893cb0)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/svg/animation/SMILTimeContainer.cpp:219
#20 0x00007ffff4c03ae7 in WebCore::Timer<WebCore::SMILTimeContainer>::fired (this=0x893ce0)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/Timer.h:114
#21 0x00007ffff47b2724 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x6e44b0)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/ThreadTimers.cpp:129
#22 0x00007ffff47b2611 in WebCore::ThreadTimers::sharedTimerFired () at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/ThreadTimers.cpp:105
#23 0x00007ffff4aac8e8 in WebCore::SharedTimerQt::timerEvent (this=0x6e44e0, ev=0x7fffffffd700)
    at /home/reni/Data/REPOS/webkit_sec/Source/WebCore/platform/qt/SharedTimerQt.cpp:113
#24 0x00007ffff221266c in QObject::event(QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#25 0x00007ffff3058dbc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Widgets.so.5
#26 0x00007ffff305c075 in QApplication::notify(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Widgets.so.5
#27 0x00007ffff21ecdbe in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#28 0x00007ffff223375c in QTimerInfoList::activateTimers() () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#29 0x00007ffff2234094 in ?? () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#30 0x00007fffee3790a6 in g_main_dispatch (context=0x6632f0) at /build/buildd/glib2.0-2.37.3/./glib/gmain.c:3058
#31 g_main_context_dispatch (context=context@entry=0x6632f0) at /build/buildd/glib2.0-2.37.3/./glib/gmain.c:3634
---Type <return> to continue, or q <return> to quit---
#32 0x00007fffee3793f8 in g_main_context_iterate (context=context@entry=0x6632f0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>)
    at /build/buildd/glib2.0-2.37.3/./glib/gmain.c:3705
#33 0x00007fffee37949c in g_main_context_iteration (context=0x6632f0, may_block=1) at /build/buildd/glib2.0-2.37.3/./glib/gmain.c:3766
#34 0x00007ffff22344bc in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
   from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#35 0x00007ffff21ebd3b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#36 0x00007ffff21ef120 in QCoreApplication::exec() () from /usr/local/Trolltech/Qt5/Qt-5.0.0-r40/lib/libQt5Core.so.5
#37 0x0000000000421ba0 in launcherMain (app=...) at /home/reni/Data/REPOS/webkit_sec/Tools/QtTestBrowser/qttestbrowser.cpp:49
#38 0x0000000000423680 in main (argc=2, argv=0x7fffffffdc68) at /home/reni/Data/REPOS/webkit_sec/Tools/QtTestBrowser/qttestbrowser.cpp:318
Comment 1 Renata Hodovan 2013-08-09 05:36:50 PDT
Created attachment 208420 [details]
Test case (extension needs to be .html)
Comment 2 Rob Buis 2013-08-09 09:42:10 PDT
Created attachment 208439 [details]
Patch
Comment 3 Andreas Kling 2013-08-09 11:31:27 PDT
Is this the right thing to be doing? I always figured RenderStyle::diff() skipped checking the non-SVG visited link stuff for security reasons.
Comment 4 Andreas Kling 2013-08-09 12:04:01 PDT
(In reply to comment #3)
> Is this the right thing to be doing? I always figured RenderStyle::diff() skipped checking the non-SVG visited link stuff for security reasons.

(Disclaimer: I might be talking out of my backside here.)
Comment 5 Dirk Schulze 2013-08-09 12:13:47 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > Is this the right thing to be doing? I always figured RenderStyle::diff() skipped checking the non-SVG visited link stuff for security reasons.
> 
> (Disclaimer: I might be talking out of my backside here.)

For visiti(In reply to comment #3)
> Is this the right thing to be doing? I always figured RenderStyle::diff() skipped checking the non-SVG visited link stuff for security reasons.

Is RenderStyle::diff taken into account for privacy checking? (visited link and so on?). Just curious.

stroke-* and dash-array-* properties can not be used for styling on :visitied selector.
Comment 6 Dirk Schulze 2013-08-09 12:14:55 PDT
Comment on attachment 208439 [details]
Patch

r=me
Comment 7 Philip Rogers 2013-08-09 15:52:50 PDT
Comment on attachment 208439 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208439&action=review

This looks good to me too in terms of security (post wkbug.com/119492)

> LayoutTests/svg/animations/animate-stroke-crasher.html:1
> +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

Please add <!DOCTYPE HTML> and such to make this valid.

> LayoutTests/svg/animations/animate-stroke-crasher.html:8
> +      <animate attributename="stroke"></animate>

Missing </a>
Comment 8 Rob Buis 2013-08-09 16:29:54 PDT
Committed r153914: <http://trac.webkit.org/changeset/153914>