My SVGFuzzer crashes on this svg test: <svg xmlns="http://www.w3.org/2000/svg"> <circle> <animate attributeName="display" to="bevel"></animate> </circle> </svg> The crash comes from the CSSParser. StylePropertySet::setProperty was called with 0 (what is 0 by default too) contextStyleSheet from SVGAnimateElement. This zero was propageted to CSSParser::isValidKeywordPropertyAndValue() where parserContext.isCSSGridLayoutEnabled was NULL too and caused a segfault. Backtrace: Program received signal SIGSEGV, Segmentation fault. 0x00007ffff3d05012 in isValidKeywordPropertyAndValue (propertyId=WebCore::CSSPropertyDisplay, valueID=724, parserContext=...) at /home/reni/WebKit-git/Source/WebCore/css/CSSParser.cpp:632 632 if (parserContext.isCSSGridLayoutEnabled && (valueID == CSSValueWebkitGrid || valueID == CSSValueWebkitInlineGrid)) (gdb) bt 8 #0 0x00007ffff3d05012 in isValidKeywordPropertyAndValue (propertyId=WebCore::CSSPropertyDisplay, valueID=724, parserContext=...) at /home/reni/WebKit-git/Source/WebCore/css/CSSParser.cpp:632 #1 0x00007ffff3d06119 in parseKeywordValue (declaration=0x9926f0, propertyId=WebCore::CSSPropertyDisplay, string=..., important=false, parserContext=...) at /home/reni/WebKit-git/Source/WebCore/css/CSSParser.cpp:1119 #2 0x00007ffff3d0690a in WebCore::CSSParser::parseValue (declaration=0x9926f0, propertyID=WebCore::CSSPropertyDisplay, string=..., important=false, cssParserMode=WebCore::SVGAttributeMode, contextStyleSheet=0x0) at /home/reni/WebKit-git/Source/WebCore/css/CSSParser.cpp:1232 #3 0x00007ffff3e01d11 in WebCore::StylePropertySet::setProperty (this=0x9926f0, propertyID=WebCore::CSSPropertyDisplay, value=..., important=false, contextStyleSheet=0x0) at /home/reni/WebKit-git/Source/WebCore/css/StylePropertySet.cpp:661 #4 0x00007ffff4ac098d in applyCSSPropertyToTarget (targetElement=0x98c0c0, id=WebCore::CSSPropertyDisplay, value=...) at /home/reni/WebKit-git/Source/WebCore/svg/SVGAnimateElement.cpp:242 #5 0x00007ffff4ac0b4a in applyCSSPropertyToTargetAndInstances (targetElement=0x98c0c0, attributeName=..., valueAsString=...) at /home/reni/WebKit-git/Source/WebCore/svg/SVGAnimateElement.cpp:264 #6 0x00007ffff4ac129f in WebCore::SVGAnimateElement::applyResultsToTarget (this=0x9985b0) at /home/reni/WebKit-git/Source/WebCore/svg/SVGAnimateElement.cpp:359 #7 0x00007ffff4a5e516 in WebCore::SMILTimeContainer::updateAnimations (this=0x96a280, elapsed=..., seekToTime=false) at /home/reni/WebKit-git/Source/WebCore/svg/animation/SMILTimeContainer.cpp:319 (More stack frames follow...)
Maybe we could simply extend it to if(contextSytleSheet && parseKeyWordValue()) {} in parseValue(). But I'm not really familiar with CSS. Ideas?
We get the same crash with <set> svg tag too: <svg xmlns="http://www.w3.org/2000/svg"> <rect> <set attributeName="display" to="round"></set> </rect> </svg>
Created attachment 179965 [details] Speculative fix Maybe I should add a new test case too (?).
Regressed in <http://trac.webkit.org/changeset/118044>.
Comment on attachment 179965 [details] Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=179965&action=review > Source/WebCore/ChangeLog:10 > + No new tests (OOPS!). Please add a test.
Comment on attachment 179965 [details] Speculative fix View in context: https://bugs.webkit.org/attachment.cgi?id=179965&action=review > Source/WebCore/css/CSSParser.cpp:1232 > - if (parseKeywordValue(declaration, propertyID, string, important, contextStyleSheet->parserContext())) > + if (contextStyleSheet && parseKeywordValue(declaration, propertyID, string, important, contextStyleSheet->parserContext())) I don't think this is correct. A few lines below, we create a CSSParserContext or use contextStyleSheet->parserContext() if it's available. We should try moving that code block here. This might have a perf impact. Try running run-perf-tests Dromaeo/jslib-style-prototype to check.
Created attachment 180052 [details] Proposed patch
(In reply to comment #6) > (From update of attachment 179965 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179965&action=review > > > Source/WebCore/css/CSSParser.cpp:1232 > > - if (parseKeywordValue(declaration, propertyID, string, important, contextStyleSheet->parserContext())) > > + if (contextStyleSheet && parseKeywordValue(declaration, propertyID, string, important, contextStyleSheet->parserContext())) > > I don't think this is correct. A few lines below, we create a CSSParserContext or use contextStyleSheet->parserContext() if it's available. We should try moving that code block here. This might have a perf impact. Try running run-perf-tests Dromaeo/jslib-style-prototype to check. I've runned both versions a few times and it's seems that there is no measureable difference between them (+/-1 %). This way I choosed you idea.
Comment on attachment 180052 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=180052&action=review > LayoutTests/fast/css/invalid-parsercontext-valid-keyword-crash-expected.txt:1 > +Excellent - did not crash. See bug https://bugs.webkit.org/show_bug.cgi?id=105275 Nit: Normally we write something like "This test passes if it does not crash." or just PASS, but this is also OK. > LayoutTests/fast/css/invalid-parsercontext-valid-keyword-crash.svg:10 > + testRunner.dumpAsText(); Please indent the dumpAsText() line 4 more spaces.
Committed r138141: <http://trac.webkit.org/changeset/138141>