Last Comment Bug 514960 - [1.9.0]Data from Faulting Address controls Branch Selection starting at gklayout!nsCachedStyleData::GetStyleDisplay
: [1.9.0]Data from Faulting Address controls Branch Selection starting at gklay...
Status: VERIFIED FIXED
[sg:critical]
: crash, hang, testcase, verified1.9.0.15
Product: Core
Classification: Components
Component: Layout (show other bugs)
: 1.9.0 Branch
: All All
: -- critical (vote)
: ---
Assigned To: Mats Palmgren (:mats)
:
Mentors:
https://bugzilla.mozilla.org/attachme...
Depends on: 425981
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-06 12:52 PDT by Carsten Book [:Tomcat]
Modified: 2009-12-01 03:17 PST (History)
13 users (show)
dveditz: blocking1.9.0.15+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x-
mats: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected


Attachments
stack etc (19.50 KB, text/plain)
2009-09-06 12:52 PDT, Carsten Book [:Tomcat]
no flags Details
191 patch in bug 425981 backported to 190 (21.16 KB, patch)
2009-09-07 13:10 PDT, Mats Palmgren (:mats)
roc: review+
bzbarsky: superreview+
dveditz: approval1.9.0.15+
Details | Diff | Review

Description Carsten Book [:Tomcat] 2009-09-06 12:52:16 PDT
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.
Comment 1 Daniel Veditz [:dveditz] 2009-09-06 15:45:58 PDT
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.
Comment 2 Gary Kwong [:gkw] [:nth10sd] 2009-09-06 23:31:31 PDT
(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.
Comment 3 Mats Palmgren (:mats) 2009-09-07 10:15:01 PDT
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].
Comment 4 Samuel Sidler (old account; do not CC) 2009-09-07 10:19:12 PDT
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?
Comment 5 Mats Palmgren (:mats) 2009-09-07 10:33:27 PDT
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...
Comment 6 Mats Palmgren (:mats) 2009-09-07 13:10:41 PDT
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
Comment 7 Mats Palmgren (:mats) 2009-09-07 14:42:15 PDT
TryServer builds with the attached patch:
https://build.mozilla.org/tryserver-builds/mpalmgren@mozilla.com-1252353513/
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2009-09-07 15:33:48 PDT
Comment on attachment 399102 [details] [diff] [review]
191 patch in bug 425981 backported to 190

Thanks
Comment 9 Daniel Veditz [:dveditz] 2009-09-10 10:50:22 PDT
Comment on attachment 399102 [details] [diff] [review]
191 patch in bug 425981 backported to 190

Approved for 1.9.0.15, a=dveditz for release-drivers
Comment 10 Mats Palmgren (:mats) 2009-09-10 16:28:40 PDT
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
Comment 11 Mats Palmgren (:mats) 2009-09-11 18:03:29 PDT
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 1.9.0.15 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.
Comment 12 Al Billings [:abillings] 2009-09-17 10:56:20 PDT
So, Mats, the crashtest from bug 425981 will be checked into 1.9.0 after release?
Comment 13 Al Billings [:abillings] 2009-09-17 15:18:36 PDT
Verified fixed using testcase in 1.9.0 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.15pre) Gecko/2009091321 GranParadiso/3.0.15pre (.NET CLR 3.5.30729) (my own debug build).
Comment 14 Mats Palmgren (:mats) 2009-09-18 05:45:19 PDT
(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.
Comment 15 Daniel Veditz [:dveditz] 2009-10-23 17:30:19 PDT
Doesn't appear to affect the 1.8 branch
Comment 16 Mats Palmgren (:mats) 2009-11-03 04:54:32 PST
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

Note You need to log in before you can comment on or make changes to this bug.