Closed Bug 918450 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Workers, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla27
Tracking Status
firefox25 --- unaffected
firefox26 + verified
firefox27 + verified
firefox-esr17 --- unaffected
firefox-esr24 --- unaffected
b2g18 --- unaffected
b2g-v1.1hd --- unaffected
b2g-v1.2 --- fixed

People

(Reporter: ehsan, Assigned: bzbarsky)

References

()

Details

(Keywords: csectype-uaf, sec-critical)

Attachments

(1 file)

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
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...
Yep. Verifying that the backout fixes the problem.
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 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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/8eb674a22a17
Flags: in-testsuite?
Target Milestone: --- → mozilla27
https://hg.mozilla.org/mozilla-central/rev/8eb674a22a17
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → 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
Group: core-security
You need to log in before you can comment on or make changes to this bug.