514960 - [1.9.0]Data from Faulting Address controls Branch Selection starting at gklayout!nsCachedStyleData::GetStyleDisplay

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: 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]]

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] (NOT official MoCo now)]

(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 (inactive)]

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)]

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 (inactive)]

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 (inactive)]

Attachment: 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 (inactive)]

TryServer builds with the attached patch:
https://build.mozilla.org/tryserver-builds/[e-mail]-1252353513/

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 399102 [details] [diff] [review]
191 patch in bug 425981 backported to 190

Thanks

Comment 9

[Daniel Veditz [:dveditz]]

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 (inactive)]

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 (inactive)]

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 - ex-MoCo]]

So, Mats, the crashtest from bug 425981 will be checked into 1.9.0 after release?

Comment 13

[Al Billings [:abillings - ex-MoCo]]

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 (inactive)]

(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]]

Doesn't appear to affect the 1.8 branch

Comment 16

[Mats Palmgren (inactive)]

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