Closed Bug 384037 Opened 13 years ago Closed 11 years ago

Crash [@ nsIFrame::IsBoxFrame] with <xul:splitter>

Categories

(Core :: XUL, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: roc)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase, verified1.9.1, Whiteboard: [sg:dos] null deref)

Crash Data

Attachments

(2 files)

No description provided.
Severity: normal → critical
The cause of this problem is that nsFrameNavigator::GetChildBeforeAfter assumes that the parent is a box frame, but this assumption is not true in this case (it isn't even a XUL frame).

Perhaps if mParent is null we can just return nsnull from nsFrameNavigator::GetChildBeforeAfter?
Summary: Crash [@ nsIFrame::IsBoxFrame] with <xul:splitter> → [@ nsIFrame::IsBoxFrame] with <xul:splitter>
Summary: [@ nsIFrame::IsBoxFrame] with <xul:splitter> → Crash [@ nsIFrame::IsBoxFrame] with <xul:splitter>
Seems to me that nsFrameNavigator should just use nsIFrame* everywhere. There is nothing box-specific about it.
I think we should actually just add an early return in nsSplitterFrame::UpdateState (and maybe a couple other places) if mParentBox is null.  A splitter can't really work anyway without a box parent and box siblings b because of the dependencies on box layout and box attributes.  (It seems kind of weird to me that the splitter code is messing around with attributes on random content nodes, though; it could have very odd effects in some cases.  Probably something to consider changing for XUL2.)

We should also probably get rid of nsFrameNavigator; it's only used in nsSplitterFrame, and all the functions can easily be written in one line using  nsFrameList (a couple more if you care about whether the frames are boxes).
Still crashes [@ nsIFrame::IsBoxFrame] on trunk.
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
(I tested this on latest-trunk mozilla-central nightly build on WinXP SP3)

Turning security-sensitive and nominating blocking1.9.1? just-in-case, due to !exploitable PROBABLY_EXPLOITABLE result.

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
Exception Faulting Address: 0x0
First Chance Exception Type: STATUS_ACCESS_VIOLATION (0xC0000005)
Exception Sub-Type: Read Access Violation

Faulting Instruction:1013b3d0 mov eax,dword ptr [esi]

Basic Block:
    1013b3d0 mov eax,dword ptr [esi]
       Tainted Input Operands: esi
    1013b3d2 mov edx,dword ptr [eax+128h]
       Tainted Input Operands: eax
    1013b3d8 push 100h
    1013b3dd mov ecx,esi
       Tainted Input Operands: esi
    1013b3df call edx
       Tainted Input Operands: ecx, edx

Exception Hash (Major/Minor): 0x48565475.0x5e0f3b47

Stack Trace:
xul!nsIFrame::GetChildBox+0x0
xul!nsFrameNavigator::IndexOf+0xd
xul!nsFrameNavigator::GetChildBeforeAfter+0x14
xul!nsSplitterFrameInner::UpdateState+0x11cf64
xul!nsSplitterFrame::DoLayout+0x1f
xul!nsBoxFrame::Reflow+0x113
xul!nsLineLayout::ReflowFrame+0x8fa
xul!nsBlockFrame::ReflowInlineFrame+0x31
xul!nsBlockFrame::DoReflowInlineFrames+0x1a6
xul!nsBlockFrame::ReflowInlineFrames+0x188
xul!nsBlockFrame::ReflowLine+0x74
xul!nsBlockFrame::ReflowDirtyLines+0x171
xul!nsBlockFrame::Reflow+0x1e2
xul!nsBlockReflowContext::ReflowBlock+0xd4
xul!nsBlockFrame::ReflowBlockFrame+0x43d
xul!nsBlockFrame::ReflowLine+0x15d
xul!nsBlockFrame::ReflowDirtyLines+0x171
xul!nsBlockFrame::Reflow+0x1e2
xul!nsContainerFrame::ReflowChild+0x6c
xul!CanvasFrame::Reflow+0x11c
xul!nsContainerFrame::ReflowChild+0x6c
xul!nsHTMLScrollFrame::ReflowScrolledFrame+0x19a
xul!nsHTMLScrollFrame::ReflowContents+0x7b
xul!nsHTMLScrollFrame::Reflow+0x1d9
xul!nsContainerFrame::ReflowChild+0x6c
xul!ViewportFrame::Reflow+0x11c
xul!PresShell::DoReflow+0x249
xul!PresShell::ProcessReflowCommands+0x11e
xul!PresShell::DoFlushPendingNotifications+0x12f
xul!PresShell::ReflowEvent::Run+0x37
xul!nsThread::ProcessNextEvent+0x213
xul!nsBaseAppShell::Run+0x4a
xul!nsAppStartup::Run+0x1e
xul!XRE_main+0xcb9
Unknown
Instruction Address: 0x1013b3d0

Description: Data from Faulting Address controls Code Flow
Short Description: TaintedDataControlsCodeFlow
Exploitability Classification: PROBABLY_EXPLOITABLE
Recommended Bug Title: Probably Exploitable - Data from Faulting Address controls Code Flow starting at xul!nsIFrame::GetChildBox+0x0 (Hash=0x48565475.0x5e0f3b47)

The data from the faulting address is later used as the target for a branch.
Group: core-security
Flags: blocking1.9.1?
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [sg:critical?]
Assignee: nobody → roc
Attached patch fixSplinter Review
This is more or less what Eli suggested. Gets rid of nsFrameNavigator. Replaces its usage with nsFrameList. Sprinkles a few IsBoxFrame checks so that we bail out if our sibling or parent is not a XUL box.

FWIW this bug is a pure null deref, as far as I can tell, so the "!exploitable" tool is not giving us a good assessment here.
Attachment #370174 - Flags: superreview?(dbaron)
Attachment #370174 - Flags: review?(dbaron)
Whiteboard: [sg:critical?] → [needs review]
Flags: blocking1.9.1? → wanted1.9.1+
Comment on attachment 370174 [details] [diff] [review]
fix

r+sr=dbaron; sorry for the delay.  This was simpler than I'd expected it to be.
Attachment #370174 - Flags: superreview?(dbaron)
Attachment #370174 - Flags: superreview+
Attachment #370174 - Flags: review?(dbaron)
Attachment #370174 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/d56efd447561
Group: core-security
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs review]
1.9.0 and 1.9.1 !exploitable report:
PROBABLY_EXPLOITABLE: Probably Exploitable - Data from Faulting Address controls Code Flow starting at gklayout!nsIFrame::IsBoxFrame
Flags: blocking1.9.0.11?
Flags: blocking1.9.0.11? → wanted1.9.0.x+
Whiteboard: [sg:dos] null deref
Attachment #370174 - Flags: approval1.9.1? → approval1.9.1+
Whiteboard: [sg:dos] null deref → [sg:dos] null deref [needs 191 landing]
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/dec49167c08c
Keywords: fixed1.9.1
Whiteboard: [sg:dos] null deref [needs 191 landing] → [sg:dos] null deref
This caused a regression: see bug 491323
Verified fixed on the 1.9.1 branch using:

->Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090522 Shiretoko/3.5pre

->Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090522 Shiretoko/3.5pre

->Win Vista equivalent nightly
Crash Signature: [@ nsIFrame::IsBoxFrame]
You need to log in before you can comment on or make changes to this bug.