Crash [@ nsEventStateManager::GetContentState] [@ nsNativeTheme::CheckBooleanAttr] with -moz-appearance and position: fixed

VERIFIED FIXED in mozilla1.9.2a1

Status

()

Core
Widget
P2
critical
VERIFIED FIXED
11 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: vlad)

Tracking

(Blocks: 1 bug, 6 keywords)

Trunk
mozilla1.9.2a1
crash, regression, testcase, verified1.9.0.11, verified1.9.0.12, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
blocking1.9.0.11 +
blocking1.9.0.12 +
wanted1.9.0.x +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
Created attachment 264443 [details]
testcase (crashes Firefox when loaded)

Loading the testcase in Firefox trunk on Mac (opt or debug) makes it crash.
Created attachment 264467 [details]
Frame dump + stack

The native theme code makes bad assumptions about the frame tree...
Assignee: general → mats.palmgren
Status: NEW → ASSIGNED
OS: Mac OS X → All
Summary: Crash [@ nsEventStateManager::GetContentState] with -moz-appearance and position: fixed → Crash [@ nsEventStateManager::GetContentState] [@ nsNativeTheme::CheckBooleanAttr] with -moz-appearance and position: fixed
Created attachment 264528 [details] [diff] [review]
Patch rev. 1

Add null-checks.
Attachment #264528 - Flags: superreview?(roc)
Attachment #264528 - Flags: review?(roc)
How can a null frame, or a frame with null content, get passed in to native theme APIs?
Comment on attachment 264528 [details] [diff] [review]
Patch rev. 1

clearing request pending comments
Attachment #264528 - Flags: superreview?(roc)
Attachment #264528 - Flags: review?(roc)
Duplicate of this bug: 429311
(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:1019945f mov eax,dword ptr [esi]

Basic Block:
    1019945f mov eax,dword ptr [esi]
       Tainted Input Operands: esi
    10199461 mov edx,dword ptr [eax+3ch]
       Tainted Input Operands: eax
    10199464 push 80h
    10199469 mov ecx,esi
       Tainted Input Operands: esi
    1019946b call edx
       Tainted Input Operands: ecx, edx

Exception Hash (Major/Minor): 0x6c0b5600.0xb3e226e

Stack Trace:
xul!nsNativeTheme::CheckBooleanAttr+0xf
xul!nsNativeTheme::GetCheckedOrSelected+0x33c0dc
xul!nsNativeThemeWin::GetThemePartAndState+0xf0
xul!nsNativeThemeWin::GetMinimumWidgetSize+0x23cf14
xul!nsCSSOffsetState::InitOffsets+0x184
xul!nsCOMPtr_base::assign_from_qi+0x19
xul!nsAbsoluteContainingBlock::Reflow+0x132
xul!ViewportFrame::Reflow+0x1a0
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: 0x1019945f

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!nsNativeTheme::CheckBooleanAttr+0xf (Hash=0x6c0b5600.0xb3e226e)

The data from the faulting address is later used as the target for a branch.
Group: core-security
Flags: blocking1.9.1?
QA Contact: general
Whiteboard: [sg:critical?]
Hardware: x86 → All
Any response to roc's question?
So the aFrame passed in to nsNativeTheme::GetContentState has a mContent, but there's this chunk of code:

77	  PRBool isXULCheckboxRadio = 
78	    (aWidgetType == NS_THEME_CHECKBOX ||
79	     aWidgetType == NS_THEME_RADIO) &&
80	    aFrame->GetContent()->IsNodeOfType(nsINode::eXUL);
81	  if (isXULCheckboxRadio)
82	    aFrame = aFrame->GetParent();

isXULCheckboxRadio is TRUE, so we grab the frame's parent.  The frame's parent doesn't have mContent, and has a mRect of 0,0,65820,59880 (which I think is 2194x1996 pixels?)  No idea what type it is.

We grab this parent frame for checkbox/radio in a few different places.

roc, any ideas as to what we should do here?
oof, it's the viewport. that makes sense I guess.

Where we do
  if (isXULCheckboxRadio)
    aFrame = aFrame->GetParent();
we should check that the new aFrame's content is non-null and bail if it is.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Created attachment 372441 [details] [diff] [review]
patch

This fixes the testcase for me; just protects the places where we grab a parent and then grab its content.
Assignee: mats.palmgren → vladimir
Attachment #372441 - Flags: review?(roc)
We'll want the crashtest checked in of course...
Hmm.. I'll hold off on checking in the crashtest actually, since this bug is old enough that it might be in 3.0.x.  Sam, do you know if that's the case?
Flags: in-testsuite?
Fixed on trunk and 1.9.1.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
This, indeed, crashes 1.9.0. I tested with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.10pre) Gecko/2009041404 GranParadiso/3.0.10pre
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.10?
If this is fixed by a simple null-check is it really sg:critical, or only sg:dos?
Flags: blocking1.9.0.10? → blocking1.9.0.10+
Comment on attachment 372441 [details] [diff] [review]
patch

Approved for 1.9.0.10, a=dveditz for release-drivers
Attachment #372441 - Flags: approval1.9.0.10+
Verfied fixed on trunk and 1.9.1 with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090421 Minefield/3.6a1pre ID:20090421032809

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090421 Shiretoko/3.5b4pre ID:20090421030848
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Target Milestone: --- → mozilla1.9.2a1
checked in on 1.9.0.10
Keywords: fixed1.9.0.10

Comment 19

9 years ago
Verified for 1.9.0.11 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.11pre) Gecko/2009051111 GranParadiso/3.0.11pre. No crash with testcase.
Keywords: fixed1.9.0.11 → verified1.9.0.11
The testcase still crashes on Linux, there's a missing null pointer check at nsNativeThemeGTK::GetGtkWidgetAndState():

#0  0x012a244e in nsNativeThemeGTK::GetGtkWidgetAndState (this=0xb5619000, aWidgetType=3 '\003', aFrame=0xaf5284ac, aGtkWidgetType=@0xbfced668,	aState=0xbfced66c, aWidgetFlags=0xbfced664) at nsNativeThemeGTK.cpp:247

246
247  if (aFrame && aFrame->GetContent()->IsNodeOfType(nsINode::eXUL)) { 
248    // For these widget types, some element (either a child or parent)
249    // actually has element focus, so we check the focused attribute
250    // to see whether to draw in the focused state.
251    if (aWidgetType == NS_THEME_TEXTFIELD || 

(gdb) p	aFrame->mContent
$5 = (class nsIContent *) 0x0
(In reply to comment #20)
> The testcase still crashes on Linux

Forgot to mention, it's 1.9.0.11.
qawanted: Al, please check out Martin's claim in comment 21. If this is not fixed completely we need to pull it out of the advisories and finish in 1.9.0.12

Does not affect the 1.8 branch (tested on Mac 1.8.1.22pre)
Flags: wanted1.8.1.x-
Flags: blocking1.9.0.12?
Keywords: verified1.9.0.11 → fixed1.9.0.11, qawanted, regression

Comment 23

9 years ago
Ack. It does crash on Linux with 1.9.0.11. 

This was listed as a Mac crash in comments so I had no reason to specifically look on Linux before now once it was fixed on Mac. Weird.
Does this crash on 1.9.1 on Linux too?

Comment 25

9 years ago
Yes, it crashes last night's 1.9.1.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090529 Shiretoko/3.5pre
I'm going to assume trunk is still crashing too then and reopen this. "yay"

We need to be sure this bug isn't listed in our advisories as well.
Status: VERIFIED → REOPENED
Keywords: fixed1.9.0.11, verified1.9.1
Resolution: FIXED → ---
Keywords: qawanted
As per comments 20-26, marking this as Linux only (has anyone tested it on w32?). Vlad: do you still think this blocks?
OS: All → Linux
This doesn't crash for me on windowsxp, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.12pre) Gecko/2009052005 GranParadiso/3.0.12pre (.NET CLR 3.5.30729)
My temptation is to unblock on this, but if all it needs is a nullcheck, can we get a patch whipped together?
Created attachment 380868 [details] [diff] [review]
linux 1.9.0 patch

This is on the try server now; linux 1.9.0 patch.
Attachment #380868 - Flags: review?(roc)
Created attachment 380869 [details] [diff] [review]
linux 1.9.1 patch

and the same patch ported to 1.9.1
Attachment #380869 - Flags: review?(roc)
Created attachment 380871 [details] [diff] [review]
linux trunk patch

and the same patch for the trunk, all slightly different!
All the patches had a compilation bug.  That's what I get for not using emacs.
+  nsIContent *content = aFrame ? nsnull : aFrame->GetContent();

So if aFrame is non-null, we set content to null, otherwise aFrame is null and we dereference it?
Flags: blocking1.9.0.12? → blocking1.9.0.12+
There's a tryserver build going of 190 with the fixed patch; I don't have linux easily accessible here, I'll post a link here when it's done.   Then I'll kick off 191 and 192 builds, given that the patches are slightly different for all three.

Comment 37

9 years ago
Vlad, I tested your build on Ubuntu 8.10. It doesn't crash anymore. Firefox 3.0.10 on the same machine does.
Can you attach the fixed patch?
Created attachment 380989 [details] [diff] [review]
updated patch, 1.9.0

Updated patch, the 1.9.0 flavour; the others are similar.
Attachment #380868 - Attachment is obsolete: true
Attachment #380869 - Attachment is obsolete: true
Attachment #380871 - Attachment is obsolete: true
Attachment #380989 - Flags: review?(roc)
Attachment #380868 - Flags: review?(roc)
Attachment #380869 - Flags: review?(roc)
Attachment #380871 - Flags: review?(roc)
Checking in nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.161; previous revision: 1.160
done

http://hg.mozilla.org/mozilla-central/rev/78c6f6a6ecb5
http://hg.mozilla.org/mozilla-central/rev/2b3ff1a9ad49


http://hg.mozilla.org/releases/mozilla-1.9.1/rev/60e46f62c924

marking fixed 1.9.0.12, not sure if 11 or 12 is right.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Keywords: fixed1.9.0.12, fixed1.9.1
Resolution: --- → FIXED
Comment on attachment 380989 [details] [diff] [review]
updated patch, 1.9.0

/me subtly notes that patches on 1.9.0 need explicit approval before landing and retroactively approves.

Approved for 1.9.0.12. a=ss
Attachment #380989 - Flags: approval1.9.0.12+
Fix checked into 1.9.0.11 relbranch

Checking in nsNativeThemeGTK.cpp;
/cvsroot/mozilla/widget/src/gtk2/nsNativeThemeGTK.cpp,v  <--  nsNativeThemeGTK.cpp
new revision: 1.160.14.1; previous revision: 1.160
Keywords: fixed1.9.0.11

Comment 43

9 years ago
Verified for 1.9.0.11 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.11) Gecko/2009060214 Firefox/3.0.11.
Keywords: fixed1.9.0.11 → verified1.9.0.11
Sorry that I missed to verify the fix on Linux. But now I can also verify that it is fixed on trunk and 1.9.1 with

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090602 Minefield/3.6a1pre ID:20090602031649

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1pre) Gecko/20090603 Shiretoko/3.5pre ID:20090603031326
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
OS: Linux → All
Group: core-security
(Reporter)

Comment 45

9 years ago
Crashtest added:

http://hg.mozilla.org/mozilla-central/rev/bf3a4f5dd798
Flags: in-testsuite? → in-testsuite+

Comment 46

9 years ago
Verified again for 1.9.0.12 with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.12pre) Gecko/2009070104 GranParadiso/3.0.12pre.
Keywords: fixed1.9.0.12 → verified1.9.0.12
Crash Signature: [@ nsEventStateManager::GetContentState] [@ nsNativeTheme::CheckBooleanAttr]
You need to log in before you can comment on or make changes to this bug.