Crash in dispatching mozilla::dom::workers::EventListenerManager::DispatchEvent

VERIFIED FIXED in Firefox 26, Firefox OS v1.2

Status

()

Core
DOM: Workers
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: Ehsan, Assigned: bz)

Tracking

({csectype-uaf, sec-critical})

Trunk
mozilla27
x86
Mac OS X
csectype-uaf, sec-critical
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox25 unaffected, firefox26+ verified, firefox27+ verified, firefox-esr17 unaffected, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.1hd unaffected, b2g-v1.2 fixed)

Details

(URL)

Attachments

(1 attachment)

Ehsan, thank you!  This is almost certainly the same thing as bug 916860... but I can reproduce this one.

So we land in mozilla::dom::workers::EventListenerManager::DispatchEvent and on this line:

390         if (!JS_ValueToObject(aCx, listenerVal, listenerObj.address())) {

here listenerVal is dead:

(gdb) p listenerVal.toObject().slots
$8 = ('js::HeapSlot' *) 0xdadadadadadadada
(gdb) p *listenerVal.toObject().getClass()
$10 = {
  name = 0x144a92b80 "@,�D\001", 
  flags = 1151937408, 
  addProperty = 0xdadadadadadadada, 
  delProperty = 0xdadadadadadadada, 
  getProperty = 0xdadadadadadadada, 

etc.
Blocks: 916860
Group: core-security
And, unsurprisingly, collection->mListeners.getFirst()->mListener.ptr is the same dead object.
I can reproduce this bug even if I back out bug 914334.
So fwiw, if I build rev e5ca10a2b3d0 (the start of the range in bug 916860) this bug goes away. So bisecting now.
The first bad revision is:
changeset:   146382:f4804e82856f
user:        Andrew McCreight <amccreight@mozilla.com>
date:        Tue Sep 10 08:29:44 2013 -0700
summary:     Bug 911333 - Remove customTrace from bindings codegen. r=bz

I suck.  We thought this was unused, but this part is key:

-        self.customTrace = desc.get('customTrace', self.workers)
Blocks: 911333
tracking-firefox26: --- → ?
Assignee: khuey → nobody
D'oh.  I noticed that bit of code and somehow didn't put it together.  I think the right thing to do is to just back out bug 911333 from Aurora and Nightly...
status-b2g18: --- → unaffected
status-firefox25: --- → unaffected
status-firefox26: --- → affected
status-firefox27: --- → affected
status-firefox-esr17: --- → unaffected
status-firefox-esr24: --- → unaffected
tracking-firefox27: --- → ?
Keywords: csec-uaf, sec-critical
Yep. Verifying that the backout fixes the problem.
Created attachment 807561 [details] [diff] [review]
Back out the fix for bug 911333, since trace hooks are in fact used in workers.
Attachment #807561 - Flags: review?(khuey)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment on attachment 807561 [details] [diff] [review]
Back out the fix for bug 911333, since trace hooks are in fact used in workers.

Review of attachment 807561 [details] [diff] [review]:
-----------------------------------------------------------------

Clearly we need a test for this too.
Attachment #807561 - Flags: review?(khuey) → review+
Comment on attachment 807561 [details] [diff] [review]
Back out the fix for bug 911333, since trace hooks are in fact used in workers.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?  Not sure.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?  Not quite.

Which older supported branches are affected by this flaw?  Aurora 26.

If not all supported branches, which bug introduced the flaw?  Bug 911333.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?  This patch should apply to Aurora 26.

How likely is this patch to cause regressions; how much testing does it need?  Not likely to cause problems.
Attachment #807561 - Flags: sec-approval?
Comment on attachment 807561 [details] [diff] [review]
Back out the fix for bug 911333, since trace hooks are in fact used in workers.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 911333.
User impact if declined: Crashes in worker code.
Testing completed (on m-c, etc.): Fixes the testcase here.
Risk to taking this patch (and alternatives if risky): Very low risk
String or IDL/UUID changes made by this patch: None.
Attachment #807561 - Flags: approval-mozilla-aurora?

Comment 12

5 years ago
Comment on attachment 807561 [details] [diff] [review]
Back out the fix for bug 911333, since trace hooks are in fact used in workers.

Sec-approval+ for trunk and approval for Aurora after it goes into trunk and things are green.
Attachment #807561 - Flags: sec-approval?
Attachment #807561 - Flags: sec-approval+
Attachment #807561 - Flags: approval-mozilla-aurora?
Attachment #807561 - Flags: approval-mozilla-aurora+

Updated

5 years ago
tracking-firefox26: ? → +
tracking-firefox27: ? → +
https://hg.mozilla.org/mozilla-central/rev/8eb674a22a17
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/cf30adef42d3
status-b2g-v1.1hd: --- → unaffected
status-b2g-v1.2: --- → fixed
status-firefox26: affected → fixed
status-firefox27: affected → fixed
Should bug 911333 be marked WONTFIX?
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #16)
> Should bug 911333 be marked WONTFIX?

No, I can still do a more limited version.  I'll probably make the custom trace hook thing worker-only.  I also need to investigate what workers are doing with them, as that isn't really the CC way of doing things.
This signature is no longer present on Fx26 and Fx27
Status: RESOLVED → VERIFIED
status-firefox26: fixed → verified
status-firefox27: fixed → verified
Group: core-security
You need to log in before you can comment on or make changes to this bug.