1445471 - Crash in mozilla::plugins::EndpointHandler<T>::Copy<T>

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-c8717d95-f034-4ea4-a15e-a6b700180310.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll mozilla::plugins::EndpointHandler<0>::Copy<void*, unsigned __int64> dom/plugins/ipc/FunctionBroker.h:663
1 xul.dll mozilla::plugins::Marshaler<0, mozilla::plugins::RequestHandler<17, int > >::MaybeUnmarshalParameter<0, void*, 1, 0>::UnmarshalParameter<unsigned __int64> dom/plugins/ipc/FunctionBroker.h:877
2 xul.dll mozilla::plugins::FunctionBroker<7, int >::BrokerCallServer dom/plugins/ipc/FunctionBroker.h:1400
3 xul.dll mozilla::plugins::FunctionBroker<7, int >::RunOriginalFunction dom/plugins/ipc/FunctionBroker.h:1252
4 xul.dll mozilla::plugins::FunctionBrokerParent::RecvBrokerFunction dom/plugins/ipc/FunctionBrokerParent.cpp:111
5 xul.dll mozilla::plugins::PFunctionBrokerParent::OnMessageReceived ipc/ipdl/PFunctionBrokerParent.cpp:137
6 xul.dll mozilla::ipc::MessageChannel::DispatchSyncMessage ipc/glue/MessageChannel.cpp:2078
7 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW ipc/glue/MessageChannel.cpp:2036
8 xul.dll mozilla::ipc::MessageChannel::RunMessage ipc/glue/MessageChannel.cpp:1886
9 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1919

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

this browser crash signature is newly showing up in firefox 60 with MOZ_RELEASE_ASSERT(aDest) that got added in bug 1382251.

Comment 1

[David Parks [:handyman]]

Attachment: Recognize invalid HANDLEs in plugin brokering

Not as deep as it initially looked.  This appears to be about InternetOpenA failing in a previous call but mapping a NULL HINSTANCE to a obfuscated ID that looks valid but isn't... so we get this crash.  No STR but this is the likely cause.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20e3e7bd1e27
Recognize invalid HANDLEs in plugin brokering. r=jimm

Comment 3

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/20e3e7bd1e27

Comment 4

[[:philipp]]

hi, could you request an uplift of the patch to 60 if you deem fit to do so?

Comment 5

[David Parks [:handyman]]

Comment on attachment 8958978 [details] [diff] [review]
Recognize invalid HANDLEs in plugin brokering

Agreed this looks good for beta

Approval Request Comment
[Feature/Bug causing the regression]:
bug 1382251

[User impact if declined]:
occasional crashes when using Flash player networking

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
yes

[Needs manual test from QE? If yes, steps to reproduce]: 
No

[List of other uplifts needed for the feature/fix]:
None

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
The code change recognizes an error that would inevitably (and quickly) lead to a browser crash (main process).  The only way this could be worse is if it created a security hole, which doesn't seem possible.

[String changes made/needed]:
N/A

Comment 6

[Julien Cristau [:jcristau]]

Comment on attachment 8958978 [details] [diff] [review]
Recognize invalid HANDLEs in plugin brokering

crash fix related to plugin ipc, verified in nightly, beta60+

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/382437b9db02

Comment 8

[[:philipp]]

60.0b6 was just released and there's still a crash with this signature - this time with MOZ_RELEASE_ASSERT(IsOdd(aSrc)): bp-6711138b-da73-43d6-a642-9e72d0180323

Comment 9

[David Parks [:handyman]]

Looks so far to be just the one crash.  And it looks like the current patch fixes _most_ of the crashes so I'm going to leave it as is for now.  It'll be a few days before I get to this (in a new bug).