Bug 20512 – Invalid CSS code crashes Safari

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Robert Swiecki]

Hi,

http://alt.swiecki.net/s-crash6.html crashes safari 3.1.2 (webkit: 35094) - sometimes it requires a few Ctrl+R

Stackdumps:
(994.1188): Access violation - code c0000005 (!!! second chance !!!)
eax=7a420000 ebx=7fbabae0 ecx=00038276 edx=00000000 esi=7ff97fe0 edi=7a80c500
eip=781473c6 esp=0012db7c ebp=0012db84 iopl=0         nv up ei pl nz na po nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000202
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\WinSxS\x86_Microsoft.VC80.CRT_1fc8b3b9a1e18e3b_8.0.50727.1433_x-ww_5cf844d2\MSVCR80.dll - 
MSVCR80!strnicmp+0x16b:
781473c6 660f6f5620      movdqa  xmm2,xmmword ptr [esi+20h] ds:0023:7ff98000=????????????????????????????????
0:000> kb
ChildEBP RetAddr  Args to Child              
WARNING: Stack unwind information not available. Following frames may be wrong.
0012db84 78147476 7a420000 7fbabae0 02000000 MSVCR80!strnicmp+0x16b
0012dbb4 0086cd04 7a420000 7fbabae0 02000000 MSVCR80!strnicmp+0x21b
0012dbd4 0086cfc9 7fbabae0 01000000 7f8e0a80 WebKit!WebCore::StringImpl::StringImpl+0x34 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\platform\text\stringimpl.cpp @ 80]
0012dbe8 0086aab2 0012dc08 7fbabae0 01000000 WebKit!WebCore::StringImpl::create+0x29 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\platform\text\stringimpl.cpp @ 1019]
0012dc00 00808d9e 7fbabae0 01000000 0080e5e2 WebKit!WebCore::String::String+0x22 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\platform\text\string.cpp @ 50]
0012dc0c 0080e5e2 0012dc2c 7f8e0a80 7fc34a60 WebKit!WebCore::CSSParserString::operator WebCore::String+0xe [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssparservalues.h @ 36]
0012dc30 008094cd 0012dcb4 00000000 0012f4f0 WebKit!WebCore::CSSParser::parseCounterContent+0x32 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssparser.cpp @ 2659]
0012dcec 007e27d2 0012f4f0 0000040c 00000000 WebKit!WebCore::CSSParser::parseContent+0x16d [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssparser.cpp @ 1973]
0012deb0 00a29eac 0000040c 00000000 7f8bf7c0 WebKit!WebCore::CSSParser::parseValue+0xf72 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssparser.cpp @ 619]
0012f4d4 007658b6 0012f4f0 007658f9 0012f650 WebKit!cssyyparse+0x1e3c [c:\home\buildbot\slave\win32-~3\build\openso~1\webcore\css\cssgrammar.y @ 1212]
0012f4dc 007658f9 0012f650 7f8d1bf0 7f8d1bf0 WebKit!WebCore::CSSParser::parseSheet+0x26 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssparser.cpp @ 225]
0012f5c8 007630b5 0012f650 00000000 7f8d1ba0 WebKit!WebCore::CSSStyleSheet::parseString+0x29 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\css\cssstylesheet.cpp @ 159]
0012f618 00801a74 7f8d1bec 7f8d1ba0 0012f650 WebKit!WebCore::StyleElement::createSheet+0x115 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\dom\styleelement.cpp @ 93]
0012f648 0080930f 7f8e2390 7f8e2420 7f8e2420 WebKit!WebCore::StyleElement::process+0x84 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\dom\styleelement.cpp @ 70]
0012f658 0078cafe 7feb7b28 7fef641c 7f8e2420 WebKit!WebCore::HTMLStyleElement::finishParsingChildren+0xf [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmlstyleelement.cpp @ 56]
0012f684 0078c940 7f8d1ba0 7fef641c 7feb7b01 WebKit!WebCore::HTMLParser::popBlock+0xbe [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmlparser.cpp @ 1341]
0012f6a0 00795456 7febf93c 7fef6400 7fef6414 WebKit!WebCore::HTMLParser::processCloseTag+0x60 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmlparser.cpp @ 933]
0012f6c0 00794d18 0012f704 7fef6414 7fef6400 WebKit!WebCore::HTMLParser::parseToken+0x226 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmlparser.cpp @ 211]
0012f704 00794857 0012f744 7fef6d68 7fef641c WebKit!WebCore::HTMLTokenizer::processToken+0xa8 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmltokenizer.cpp @ 1918]
0012f738 00794302 0012f7ac 00000100 7fef6410 WebKit!WebCore::HTMLTokenizer::parseSpecial+0x447 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\webcore\html\htmltokenizer.cpp @ 364]



eax=7ffdf000 ebx=00000000 ecx=0012d6a8 edx=0012d6b0 esi=7c90de50 edi=00000003
eip=7c90e4f4 esp=0012d700 ebp=0012d7fc iopl=0         nv up ei pl zr na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000246
ntdll!KiFastSystemCallRet:
7c90e4f4 c3              ret
0:000> kb
ChildEBP RetAddr  Args to Child              
0012d6fc 7c90de5c 7c81cab6 ffffffff 00000003 ntdll!KiFastSystemCallRet
0012d700 7c81cab6 ffffffff 00000003 00000000 ntdll!ZwTerminateProcess+0xc
0012d7fc 7c81cb0e 00000003 77e8f3b0 ffffffff kernel32!_ExitProcess+0x62
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\WINDOWS\WinSxS\x86_Microsoft.VC80.CRT_1fc8b3b9a1e18e3b_8.0.50727.1433_x-ww_5cf844d2\MSVCR80.dll - 
0012d810 78131720 00000003 78131a04 00000003 kernel32!ExitProcess+0x14
WARNING: Stack unwind information not available. Following frames may be wrong.
0012d854 78131a5c 00000003 00000001 00000000 MSVCR80!amsg_exit+0x5e
0012d90c 7c9101bb 0000040f 00000000 0000040f MSVCR80!exit+0xd
*** ERROR: Symbol file could not be found.  Defaulted to export symbols for C:\Program Files\Safari\CoreGraphics.dll - 
0012db3c 6611d44d 000003d8 00001000 0012db68 ntdll!RtlAllocateHeap+0xeac
0012db50 00803365 00001000 00c47678 00c4cffc CoreGraphics!CGColorSpaceCreateWithDC+0x4ad
0012db68 008033b4 0012db84 00c47678 000cc4a3 WebKit!TCMalloc_SystemAlloc+0x55 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\javascriptcore\wtf\tcsystemalloc.cpp @ 371]
0012db8c 007baa81 000cc4a3 8f7a14e0 00000000 WebKit!WTF::TCMalloc_PageHeap::GrowHeap+0x44 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\javascriptcore\wtf\fastmalloc.cpp @ 1544]
0012db9c 00a6e587 cc4a23a8 7fbca870 0012dc30 WebKit!WTF::TCMalloc_PageHeap::New+0x51 [c:\cygwin\home\buildbot\slave\win32-release-archive\build\opensource\javascriptcore\wtf\fastmalloc.cpp @ 1241]
00000000 00000000 00000000 00000000 00000000 WebKit!WTF::fastMalloc+0x2a9f47

[Glenn Wilson]

Created attachment 22982 [details]
Test reduction for bug 20512

Attached is a possible test reduction of this issue.  I believe that this comes from the parsing of a CSS "counter" with invalid content....investigating further.

This one line of CSS by itself will crash Safari, but not IE or FF3.

[Robert Swiecki]

Then it might be a duplicate of: https://bugs.webkit.org/show_bug.cgi?id=20396

[Mark Rowe (bdash)]

<rdar://problem/6173832>

[Beth Dakin]

Created attachment 23507 [details]
Patch to prevent function input to counters

I can imagine a number of different ways to fix this bug, but here is a simple one that comes to mind. It seems like we are crashing because we are trying to convert a function to a string.

[Adam Roben (:aroben)]

Comment on attachment 23507 [details]
Patch to prevent function input to counters

Is it worthwhile to add another testcase that uses a function that WebKit's CSS parser knows about? Like -webkit-gradient or something like that?

r=me

[Beth Dakin]

Sure! I'll add one now.

[Darin Adler]

Comment on attachment 23507 [details]
Patch to prevent function input to counters

That seems a little backwards. Maybe a better check would be:

        if (i->unit != CSSPrimitiveValue::CSS_STRING)

Is there a reason to not do it that way?

[Beth Dakin]

Fixed with r36559.

[Beth Dakin]

Hmmm…good question, Darin! I am running the layout tests with your recommended change now.

[Beth Dakin]

Created attachment 23512 [details]
Better fix

Here's a better fix based on Darin's comment. Just checking against string is not sufficient since numbers are also okay.

[Beth Dakin]

Re-opening bug since I have a new patch to be reviewed.

[Darin Adler]

Comment on attachment 23512 [details]
Better fix

r=me

[Beth Dakin]

Fixed again with r36567.