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

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: Tomcat, Assigned: Mats Palmgren (vacation - back in August))

Tracking

(4 keywords)

1.9.0 Branch
crash, hang, testcase, verified1.9.0.15
Points:
---
Bug Flags:
blocking1.9.0.15 +
wanted1.9.0.x +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(status1.9.2 unaffected, status1.9.1 unaffected)

Details

(Whiteboard: [sg:critical], URL)

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
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.
Summary: Data from Faulting Address controls Branch Selection starting at gklayout!nsCachedStyleData::GetStyleDisplay → [1.9.0]Data from Faulting Address controls Branch Selection starting at gklayout!nsCachedStyleData::GetStyleDisplay
Whiteboard: [sg:critical?] fix-range-wanted
(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].
Flags: blocking1.9.0.15?
Keywords: crash
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [sg:critical?] fix-range-wanted → [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
Assignee: nobody → matspal
Attachment #399102 - Flags: review?(roc)
TryServer builds with the attached patch:
https://build.mozilla.org/tryserver-builds/mpalmgren@mozilla.com-1252353513/
Comment on attachment 399102 [details] [diff] [review]
191 patch in bug 425981 backported to 190

Thanks
Attachment #399102 - Flags: review?(roc) → review+
Attachment #399102 - Flags: approval1.9.0.15?
Depends on: 425981
status1.9.1: --- → unaffected
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.15?
Flags: blocking1.9.0.15+
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
Attachment #399102 - Flags: approval1.9.0.15? → approval1.9.0.15+
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
Attachment #399102 - Flags: superreview?(bzbarsky)

Updated

8 years ago
Attachment #399102 - Flags: superreview?(bzbarsky) → superreview+
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.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
status1.9.2: --- → unaffected
Flags: in-testsuite?
Keywords: fixed1.9.0.15
Resolution: --- → FIXED
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:1.9.0.15pre) Gecko/2009091321 GranParadiso/3.0.15pre (.NET CLR 3.5.30729) (my own debug build).
Status: RESOLVED → VERIFIED
(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.
Keywords: fixed1.9.0.15 → verified1.9.0.15
Doesn't appear to affect the 1.8 branch
Flags: wanted1.8.1.x-
Group: core-security
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
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.