Crash in nsWrapperCache::GetWrapperMaybeDead

VERIFIED FIXED in Firefox 65

Status

()

defect
--
critical
VERIFIED FIXED
4 months ago
a month ago

People

(Reporter: marcia, Assigned: smaug)

Tracking

({crash, regression, regressionwindow-wanted})

66 Branch
mozilla66
Unspecified
macOS
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 wontfix, firefox65 verified, firefox66 verified)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is
report bp-e3de76ab-519c-493d-b034-0c7a30181213.
=============================================================

Seen while looking at nightly crash stats: https://bit.ly/2QKC07v. Mac crash which as 13 crashes/5 installs in the last 7 days. No comments and the URLs don't offer much in the way of clues.

It appears this regression goes back to at least when 64 was in nightly. 

Top 10 frames of crashing thread:

0 XUL nsWrapperCache::GetWrapperMaybeDead const dom/base/nsWrapperCache.h:162
1 XUL nsWrapperCache::GetWrapper const dom/base/nsWrapperCacheInlines.h:14
2 XUL mozilla::dom::MessageBroadcaster_Binding::getChildAt dom/bindings/BindingUtils.h:959
3 XUL bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3064
4 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:443
5 XUL js::ForwardingProxyHandler::call const js/src/vm/Interpreter.cpp:606
6 XUL js::CrossCompartmentWrapper::call const js/src/proxy/CrossCompartmentWrapper.cpp:304
7 XUL js::Proxy::call js/src/proxy/Proxy.cpp:535
8 XUL js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:509
9 XUL js::jit::DoCallFallback js/src/vm/Interpreter.cpp:594

=============================================================
Olli, can you take a look here?
Flags: needinfo?(bugs)
The two crashes I see in the last week are in mozilla::dom::MessageBroadcaster_Binding::getChildAt(), like the original crash.
Component: General → DOM
All three are also cross compartment calls, so presumably that's related.
(Assignee)

Comment 4

4 months ago
bug 888600 landed already 61.

Peterv might have the webidl + MM setup better in mind.
Flags: needinfo?(bugs) → needinfo?(peterv)
(Assignee)

Comment 6

4 months ago
Most probably. At least window.messageManager.getChildAt(100); in Scratchpad (browser) crashes.
Assignee: nobody → bugs
(Assignee)

Comment 7

4 months ago
Attachment #9034083 - Flags: review?(peterv)
Attachment #9034083 - Flags: review?(peterv) → review+
(Assignee)

Updated

4 months ago
Flags: needinfo?(peterv)

Comment 8

4 months ago
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5f8f958a17f
MessageBroadcaster.getChildAt should be out-of-bounds safe, r=peterv

Comment 9

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e5f8f958a17f
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Please request Beta approval on this when you get a chance.
Flags: needinfo?(bugs)
(Assignee)

Comment 11

4 months ago
Comment on attachment 9034083 [details] [diff] [review]
mm_null_child.diff

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 888600

User impact if declined: Crash if some privileged Js code passes invalid index to MessageBroadcaster.getChildAt

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: (https://bugzilla.mozilla.org/show_bug.cgi?id=1515463#c6)

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): 

String changes made/needed:
Flags: needinfo?(bugs)
Attachment #9034083 - Flags: approval-mozilla-beta?
Comment on attachment 9034083 [details] [diff] [review]
mm_null_child.diff

[Triage Comment]
Simple null check crash fix. Approved for 65.0b9.
Attachment #9034083 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I successfully reproduced the issue on Firefox Nightly 66.0a1 (2018-12-11) under macOS 10.12 using the information found in Comment 6 and some help from Olli.

The issue is not reproducible anymore on Firefox Beta 65.0b9 and latest Nightly 66.0a1 (2019-01-08) under macOS 10.12, Windows 10 (x64) and Ubuntu 18.04 (x64).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.