crash when keypress UIEvent is triggered using dispatchEvent

VERIFIED FIXED

Status

()

defect
--
critical
VERIFIED FIXED
11 years ago
4 months ago

People

(Reporter: bhuisman, Assigned: mats)

Tracking

(4 keywords)

unspecified
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.0.4 +
wanted1.8.1.x -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse dos])

Attachments

(2 attachments)

User-Agent:       Opera/9.52 (Windows NT 5.1; U; en)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1

Firefox crashes when attempting to dispatch a keypress event created using the UIEvents type.  The following code triggers the crash:

<p>PASS if Firefox does not crash.</p>
<script type="text/javascript">
var evObj = document.createEvent('UIEvents');
evObj.initUIEvent( 'keypress', true, true, window, 1 );
document.getElementsByTagName('p')[0].dispatchEvent(evObj);
</script>

Reproducible: Always

Steps to Reproduce:
1. Create a DOM event using the UIEvents category
2. Define the event as a keypress event
3. Dispatch the event
4. Hilarity ensues
Actual Results:  
Firefox crashes

Expected Results:  
Firefox should not crash

Thanks to TarquinWJ for distilling the HTML testcase.
Keywords: crash
OS: Windows XP → All
Hardware: PC → All
Group: core-security
Posted file Reporter's testcase
Assignee: nobody → mats.palmgren
Status: UNCONFIRMED → ASSIGNED
Component: General → Event Handling
Ever confirmed: true
Keywords: testcase
Product: Firefox → Core
QA Contact: general → events
Whiteboard: [fix-in-hand]
So nsContentUtils::GetAccelKeyCandidates doesn't check that the event is really
a keyevent. That is the security problem. Regression from bug 359638.

XBL doesn't null check, that may lead to crashes too.
Posted patch Patch rev. 1Splinter Review
Null-check the result of the QI to nsIDOMKeyEvent.

Since nsContentUtils::GetAccelKeyCandidates() and some protected methods
in nsXBLWindowKeyHandler is supposed to be called with nsIDOMKeyEvent only
I changed their signatures to reflect that.
Attachment #338339 - Flags: review?(Olli.Pettay)
Attachment #338339 - Flags: review?(Olli.Pettay) → review+
Attachment #338339 - Flags: superreview?(bzbarsky)
Comment on attachment 338339 [details] [diff] [review]
Patch rev. 1

sr=bzbarsky.  Looks good.
Attachment #338339 - Flags: superreview?(bzbarsky) → superreview+
Mats, next time, can you please add a meaningful, self-contained commit message? The log should be rudimentarily readable without bugzilla available. Thanks.
Ben, I intentionally left out details about the bug since it's marked
security sensitive.  A also did not land the crash test for the same
reason (I intend to land it when the bug is made public).  I think this
is "standard procedure" for SS bugs, please correct me if I'm wrong.
Sorry, I didn't notice it's a security bug.
It's a pity to have the log messed up, it would be particularly important for security bugs. But yes, standard procedure so far.
http://hg.mozilla.org/mozilla-central/rev/5088957eb541

I'll land the crash test when the bug is public.

I can't reproduce the crash in 1.8 branch builds on Linux.

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Flags: blocking1.9.0.3?
Resolution: --- → FIXED
Whiteboard: [fix-in-hand]
Blocks: 359638
Flags: wanted1.8.1.x-
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.3+
Keywords: regression
Whiteboard: [sg:critical?]
Duplicate of this bug: CVE-2008-4324
The reporter of dupe bug 457543 has posted it to milw0rm
http://www.milw0rm.com/exploits/6614

Does this patch work for the 1.9.0 branch (cvs head)?
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080929033431 Minefield/3.1b1pre.   attached testcase does not crash.
Status: RESOLVED → VERIFIED
Comment on attachment 338339 [details] [diff] [review]
Patch rev. 1

(In reply to comment #11)
> Does this patch work for the 1.9.0 branch (cvs head)?

Yes, this patch works and is complete also for 1.9.0 branch.
Attachment #338339 - Flags: approval1.9.0.4?
Is this really potentially exploitable? We were blindly casting the wrong type of event to nsKeyEvent, but there are no virtual function calls to launch you into space and the code in question only reads incorrect data and doesn't write.
Whiteboard: [sg:critical?] → [sg:needinfo]
Comment on attachment 338339 [details] [diff] [review]
Patch rev. 1

Approved for 1.9.0.4, a=dveditz
Attachment #338339 - Flags: approval1.9.0.4? → approval1.9.0.4+
(In reply to comment #15)
> Is this really potentially exploitable? We were blindly casting the wrong type
> of event to nsKeyEvent, but there are no virtual function calls
Ah, indeed. No virtual calls, only trying to use wrong nsXXXEvent struct.
I concur with your analysis.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/base/src/nsContentUtils.cpp&rev=1.309&mark=4064-4066,4143,4145#4053
In case there is no native event we will do a QI and null-deref
crash on line 4145.
If there is a native event we access it with:

nativeKeyEvent->charCode
nativeKeyEvent->isShift
nativeKeyEvent->alternativeCharCodes.Length()
nativeKeyEvent->alternativeCharCodes[i].mUnshiftedCharCode
nativeKeyEvent->alternativeCharCodes[i].mShiftedCharCode

where 'alternativeCharCodes' is a nsTArray<nsAlternativeCharCode>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/public/nsGUIEvent.h&rev=3.162&root=/cvsroot&mark=737,712#712
and nsTArray does not have a vtable so the accesses above are "just"
out-of-bounds memory reads.
Fixed on CVS trunk for 1.9.0.4:
mozilla/content/base/public/nsContentUtils.h 	1.178
mozilla/content/base/src/nsContentUtils.cpp 	1.310
mozilla/content/xbl/src/nsXBLEventHandler.cpp 	1.75
mozilla/content/xbl/src/nsXBLWindowKeyHandler.cpp 	1.48
mozilla/content/xbl/src/nsXBLWindowKeyHandler.h 	1.16
Keywords: fixed1.9.0.4
Whiteboard: [sg:needinfo] → [sg:nse dos]
Group: core-security
Duplicate of this bug: 459230
Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102304 GranParadiso/3.0.4pre.
crash test added
http://hg.mozilla.org/mozilla-central/rev/51e405d991ce
Flags: in-testsuite? → in-testsuite+
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.