1515463 - Crash in nsWrapperCache::GetWrapperMaybeDead

Status: VERIFIED FIXED

Description

[Marcia Knous [:marcia - needinfo? me]]

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

=============================================================

Comment 1

[Andrew Overholt [:overholt]]

Olli, can you take a look here?

Comment 2

[Andrew McCreight [:mccr8]]

The two crashes I see in the last week are in mozilla::dom::MessageBroadcaster_Binding::getChildAt(), like the original crash.

Comment 3

[Andrew McCreight [:mccr8]]

All three are also cross compartment calls, so presumably that's related.

Comment 4

[Olli Pettay [:smaug]]

bug 888600 landed already 61.

Peterv might have the webidl + MM setup better in mind.

Comment 5

[Olli Pettay [:smaug]]

oh, hmm, is this just about https://searchfox.org/mozilla-central/rev/0ee0b63732d35d16ba22d5a1120622e2e8d58c29/dom/chrome-webidl/MessageManager.webidl#545 not allowing return null.

Comment 6

[Olli Pettay [:smaug]]

Most probably. At least window.messageManager.getChildAt(100); in Scratchpad (browser) crashes.

Comment 7

[Olli Pettay [:smaug]]

Attachment: mm_null_child.diff

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5f8f958a17f
MessageBroadcaster.getChildAt should be out-of-bounds safe, r=peterv

Comment 9

[Andrei Ciure[:andrei_ciure]]

https://hg.mozilla.org/mozilla-central/rev/e5f8f958a17f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 11

[Olli Pettay [:smaug]]

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:

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9034083 [details] [diff] [review]
mm_null_child.diff

[Triage Comment]
Simple null check crash fix. Approved for 65.0b9.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5c4518a4eb1f

Comment 14

[Catalin Sasca, QA [:csasca]]

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).