Closed Bug 916685 (CVE-2013-5601) Opened 10 years ago Closed 10 years ago

ASAN use-after free in GC allocation in nsEventListenerManager::SetEventHandler


(Core :: DOM: Events, defect)

Not set



Tracking Status
firefox24 --- wontfix
firefox25 + fixed
firefox26 + fixed
firefox27 + verified
firefox-esr17 --- unaffected
firefox-esr24 25+ fixed
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed


(Reporter: nils, Assigned: smaug)



(Keywords: csectype-uaf, regression, sec-critical, Whiteboard: [asan][adv-main25+][adv-esr24-1+])


(3 files)

Attached file crash.html
The attached testcase crashes the nightly ASAN build.  It require domFuzzLite3. ASAN output attached in stack.txt.
Keywords: csec-uaf
Whiteboard: [asan]
I'll take a look at this this week.
Assignee: nobody → continuation
Attachment #805180 - Attachment mime type: text/plain → text/html
Attached patch patchSplinter Review
Bring back the pre-EventHandler behavior
Assignee: continuation → bugs
Attachment #810312 - Flags: review?(bzbarsky)
Comment on attachment 810312 [details] [diff] [review]

r=me, but how are we getting a null boundHandler here?  Is the JSObjectFromInterface failing?  Seems like the only way that could happen...
Attachment #810312 - Flags: review?(bzbarsky) → review+
We end up to BindCompiledEventHandler when mIsInitialized is false. nsJSContext hasn't been
unlinked yet, so we're dealing with a context which initialization somehow failed.
Comment on attachment 810312 [details] [diff] [review]

[Approval Request Comment]
Bug caused by (feature/regressing bug #): I think Bug 807226
User impact if declined: crashes, possibly exploitable
Testing completed (on m-c, etc.):  NA
Risk to taking this patch (and alternatives if risky): Should be safe. Setting a member variable explicitly to null 
String or IDL/UUID changes made by this patch: NA
Attachment #810312 - Flags: sec-approval?
Attachment #810312 - Flags: approval-mozilla-esr24?
Attachment #810312 - Flags: approval-mozilla-beta?
Attachment #810312 - Flags: approval-mozilla-aurora?
Comment on attachment 810312 [details] [diff] [review]

sec-approval+ for trunk. As you say, this looks pretty safe so I'm giving it branch approval elsewhere for after it makes it into trunk and things are green.

Is B2G affected?
Attachment #810312 - Flags: sec-approval?
Attachment #810312 - Flags: sec-approval+
Attachment #810312 - Flags: approval-mozilla-esr24?
Attachment #810312 - Flags: approval-mozilla-esr24+
Attachment #810312 - Flags: approval-mozilla-beta?
Attachment #810312 - Flags: approval-mozilla-beta+
Attachment #810312 - Flags: approval-mozilla-aurora?
Attachment #810312 - Flags: approval-mozilla-aurora+
I have no idea which branch b2g uses these days, so I don't know whether it is affected.
b2g-18 shouldn't be.
Closed: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Hi Nils - I'm not able to recreate the crash with an ASan build from 2013-09-16 and the domFuzz extension.

Would you mind trying this on a recent build to verify that we've indeed fixed it? Any branch. Thank you.
Hi Matt, I can confirm that this testcase doesn't crash anymore in the latest ASAN build. I will keep a look out for similar crashes in my next fuzzing run.
Whiteboard: [asan] → [asan][adv-main25+][adv-esr24-1+]
Alias: CVE-2013-5601
Group: core-security
You need to log in before you can comment on or make changes to this bug.