Closed Bug 1445471 Opened 2 years ago Closed 2 years ago

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

Categories

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

60 Branch
x86_64
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: philipp, Assigned: handyman)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.
Flags: needinfo?(davidp99)
Priority: -- → P3
Flags: needinfo?(davidp99)
Priority: P3 → P1
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.
Attachment #8958978 - Flags: review?(jmathies)
Assignee: nobody → davidp99
Attachment #8958978 - Attachment description: patch1.patch → Recognize invalid HANDLEs in plugin brokering
Attachment #8958978 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/20e3e7bd1e27
Recognize invalid HANDLEs in plugin brokering. r=jimm
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/20e3e7bd1e27
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
hi, could you request an uplift of the patch to 60 if you deem fit to do so?
Flags: needinfo?(davidp99)
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
Flags: needinfo?(davidp99)
Attachment #8958978 - Flags: approval-mozilla-beta?
Comment on attachment 8958978 [details] [diff] [review]
Recognize invalid HANDLEs in plugin brokering

crash fix related to plugin ipc, verified in nightly, beta60+
Attachment #8958978 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
Flags: needinfo?(davidp99)
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).
Flags: needinfo?(davidp99)
See Also: → 1448947
You need to log in before you can comment on or make changes to this bug.