Created attachment 398970 [details] stack etc see testcase https://bugzilla.mozilla.org/attachment.cgi?id=316799 Loading this site hang Firefox 1.9.0 Debug Builds (seem not affecting 1.9.1). !exploitable report: (838.b20): Access violation - code c0000005 (first chance) First chance exceptions are reported before any exception handling. This exception may be expected and handled. eax=ddddddf9 ebx=7ffd9000 ecx=ddddddf9 edx=046771a0 esi=00011490 edi=7c911460 eip=024ab51c esp=0012ca00 ebp=0012ca08 iopl=0 nv up ei pl nz na pe nc cs=001b ss=0023 ds=0023 es=0023 fs=003b gs=0000 efl=00010206 gklayout!nsCachedStyleData::GetStyleDisplay+0xc: 024ab51c 83780400 cmp dword ptr [eax+4],0 ds:0023:ddddddfd=???????? 0:000> !exploitable -v HostMachine\HostUser Executing Processor Architecture is x86 Debuggee is in User Mode Debuggee is a live user mode debugging session on the local machine Event Type: Exception *** WARNING: Unable to verify checksum for C:\work\mozilla\builds\1.9.0\mozilla\firefox-debug\dist\bin\components\gkwidget.dll *** WARNING: Unable to verify checksum for C:\work\mozilla\builds\1.9.0\mozilla\firefox-debug\dist\bin\components\tkitcmps.dll Exception Faulting Address: 0xffffffffddddddfd First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005) Exception Sub-Type: Read Access Violation Faulting Instruction:024ab51c cmp dword ptr [eax+4],0 Basic Block: 024ab51c cmp dword ptr [eax+4],0 Tainted Input Operands: eax 024ab520 je gklayout!nscachedstyledata::getstyledisplay+0x20 (024ab530) Tainted Input Operands: ZeroFlag Exception Hash (Major/Minor): 0x3f274d05.0x73176a3a Stack Trace: gklayout!nsCachedStyleData::GetStyleDisplay+0xc gklayout!nsStyleContext::GetStyleDisplay+0x14 gklayout!nsIFrame::GetStyleDisplay+0x3b gklayout!nsLayoutUtils::GetFloatFromPlaceholder+0x37 gklayout!nsLineLayout::ReflowFrame+0x47f gklayout!nsBlockFrame::ReflowInlineFrame+0x69 gklayout!nsBlockFrame::DoReflowInlineFrames+0x1fc gklayout!nsBlockFrame::ReflowInlineFrames+0xeb gklayout!nsBlockFrame::ReflowLine+0x2cc gklayout!nsBlockFrame::ReflowDirtyLines+0x561 gklayout!nsBlockFrame::Reflow+0x28b gklayout!nsContainerFrame::ReflowChild+0xe6 gklayout!nsColumnSetFrame::ReflowChildren+0x417 gklayout!nsColumnSetFrame::Reflow+0x32e gklayout!nsBlockReflowContext::ReflowBlock+0x1a3 gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3 gklayout!nsBlockFrame::ReflowLine+0xd2 gklayout!nsBlockFrame::ReflowDirtyLines+0x561 gklayout!nsBlockFrame::Reflow+0x28b gklayout!nsBlockReflowContext::ReflowBlock+0x1a3 gklayout!nsBlockFrame::ReflowBlockFrame+0x6b3 gklayout!nsBlockFrame::ReflowLine+0xd2 gklayout!nsBlockFrame::ReflowDirtyLines+0x561 gklayout!nsBlockFrame::Reflow+0x28b gklayout!nsContainerFrame::ReflowChild+0xe6 gklayout!CanvasFrame::Reflow+0x13c gklayout!nsContainerFrame::ReflowChild+0xe6 gklayout!nsHTMLScrollFrame::ReflowScrolledFrame+0x32e gklayout!nsHTMLScrollFrame::ReflowContents+0x53 gklayout!nsHTMLScrollFrame::Reflow+0x249 gklayout!nsContainerFrame::ReflowChild+0xe6 gklayout!ViewportFrame::Reflow+0x15d gklayout!PresShell::DoReflow+0x2eb gklayout!PresShell::ProcessReflowCommands+0xf3 gklayout!PresShell::DoFlushPendingNotifications+0x189 gklayout!PresShell::ReflowEvent::Run+0x81 xpcom_core!nsThread::ProcessNextEvent+0x1fa xpcom_core!NS_ProcessNextEvent_P+0x53 gkwidget!nsBaseAppShell::Run+0x5d tkitcmps!nsAppStartup::Run+0x6b xul!XRE_main+0x2edf firefox!NS_internal_main+0x2b2 firefox!wmain+0x119 firefox!__tmainCRTStartup+0x1a6 firefox!wmainCRTStartup+0xd kernel32!BaseProcessStart+0x23 Instruction Address: 0x00000000024ab51c Description: Data from Faulting Address controls Branch Selection Short Description: TaintedDataControlsBranchSelection Exploitability Classification: UNKNOWN Recommended Bug Title: Data from Faulting Address controls Branch Selection starting at gklayout!nsCachedStyleData::GetStyleDisplay+0x000000000000000c (Hash=0x3f274d05.0x73176a3a) The data from the faulting address is later used to determine whether or not a branch is taken.
According to bug 401734 that bug was WFM on trunk/191pre by 2008-08-15. This testcase still crashed (in a new place) and was spun off into 451532, which was marked WFM March 2009. Somewhere in there is a fix for this bug. Also, we should not be unhiding "wfm" security bugs until we've validated that they indeed "wfm" on all supported branches. For a security bug "wfm" should be the start of a search for the fix, not a reason to ignore the bug thereafter.
(In reply to comment #1) > According to bug 401734 that bug was WFM on trunk/191pre by 2008-08-15. This > testcase still crashed (in a new place) and was spun off into 451532, which was > marked WFM March 2009. Somewhere in there is a fix for this bug. You mean Bug 451532 comment 4? > This seems to have regressed between 2008-06-23 and 2008-07-01. A more precise > regression range might be useful.
Crashes reliably on Linux, bp-8ae40432-c3e0-465f-a0e9-9f4032090907 It was fixed in the range 2009-01-07-02 -- 2009-01-08-02. http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-01-07+00%3A00&enddate=2009-01-08+04%3A00 The fix for bug 425981 is in that range. A local 1.9.1 Linux debug build does NOT crash. Backing out the 191 fix for bug 425981 makes it crash, with the same stack as above. We're accessing a dead frame, so marking it [sg:critical].
Mats: Can you work up a backport of the patch in bug 425981 and make sure that's all that's needed to fix this bug on the 1.9.0 branch?
Yes, I've backported the 191 patch to 190 which was straight forward. I'm reviewing the code and see if it's enough now. If all looks ok I'll attach the patch in a couple of hours...
Created attachment 399102 [details] [diff] [review] 191 patch in bug 425981 backported to 190 With this patch the first-letter frame construction code in 1.9.0 is pretty much identical to 1.9.1. FWIW, the handling of pseudo frames in ProcessChildren() is slightly different, due to bug 238072, but I don't think it should affect this patch. The patch passes reftest and crashtest with a local debug build. It's cooking on TryServer now. ProcessChildren() difference, for reference: ProcessChildren in 191: // save the incoming pseudo frame state CreateGeneratedContentFrame ConstructFrame CreateGeneratedContentFrame ProcessPseudoFrames // restore the incoming pseudo frame state WrapFramesInFirstLetter/Line ProcessChildren in 190: CreateGeneratedContentFrame // save the incoming pseudo frame state ConstructFrame ProcessPseudoFrames // restore the incoming pseudo frame state CreateGeneratedContentFrame WrapFramesInFirstLetter/Line https://bugzilla.mozilla.org/attachment.cgi?id=333217&action=diff#a/layout/base/nsCSSFrameConstructor.cpp_sec17
TryServer builds with the attached patch: https://firstname.lastname@example.org/
Comment on attachment 399102 [details] [diff] [review] 191 patch in bug 425981 backported to 190 Thanks
Comment on attachment 399102 [details] [diff] [review] 191 patch in bug 425981 backported to 190 Approved for 220.127.116.11, a=dveditz for release-drivers
Comment on attachment 399102 [details] [diff] [review] 191 patch in bug 425981 backported to 190 As a security bug, per our new policy, this patch needs explicit super-review from a second person. http://www.mozilla.org/hacking/reviewers.html
The fix landed on CVS HEAD: mozilla/layout/base/nsCSSFrameConstructor.cpp 1.1487 mozilla/layout/base/nsCSSFrameConstructor.h 1.264 Holding the crashtest until after 18.104.22.168 release. I know, it's already in the tree as part of bug 425981, but I wanted to reveal as little as possible in this checkin.
So, Mats, the crashtest from bug 425981 will be checked into 1.9.0 after release?
Verified fixed using testcase in 1.9.0 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:22.214.171.124pre) Gecko/2009091321 GranParadiso/3.0.15pre (.NET CLR 3.5.30729) (my own debug build).
(In reply to comment #12) > So, Mats, the crashtest from bug 425981 will be checked into 1.9.0 after > release? Yes, that's the plan.
Doesn't appear to affect the 1.8 branch
Checked in the crash test on CVS trunk: mozilla/layout/base/crashtests/425981-1.html 1.1 mozilla/layout/base/crashtests/crashtests.list 1.102