Block rndlnpshimswf.dll and rndlmainbrowserrecordplugin.dll

RESOLVED FIXED

Status

()

Toolkit
Blocklisting
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: Benjamin Smedberg, Assigned: dmajor (offline))

Tracking

unspecified
x86
Windows 7
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

(crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Looking through the crash stats while protected mode was turned off on beta, I discovered a crash signature which was close to 20% of Flash crashes, mozilla::plugins::child::_getproperty

These crashes appear to all be caused by the realnetworks video downloader which is hooking into Flash somehow. See e.g. https://crash-stats.mozilla.com/report/index/c18008fa-7720-41b0-b1ed-379f52150202

I think we should straight-up block this DLL, since it's destined to always be a stability problem.

Chad, can you confirm my decision here?

dmajor, please verify that our DLL blocklist works in the plugin-container process and prepare the necessary patch for blocking these two DLLs.
Flags: needinfo?(cweiner)

Comment 1

3 years ago
Confirmed
Flags: needinfo?(cweiner)
(Assignee)

Comment 2

3 years ago
This could break a real use case that some people may want. Has there been any outreach to RealNetworks for a fix from their side?

These crashes are in the latest version (17.0.15.4 from October 2014). Do you want to block all versions or just the latest? Either way has a potential downside.

From the technical side, plugin-container.exe does not currently use the blocklist. I tried hooking up a delayload mozglue but it messed up the fragile DLL balance of bug 1023941. I haven't yet figured out why.
relnote-firefox: --- → ?
Flags: needinfo?(benjamin)
(Reporter)

Comment 3

3 years ago
I don't care about the RealNetworks code working. Hitching a ride on the Flash plugin is not a good way to accomplish that use-case and there are Firefox extensions that solve the same problems without the hooking. Plus youtube is switching away from Flash in the near future anyway.
Flags: needinfo?(benjamin)
(Assignee)

Comment 4

3 years ago
So, plugin-container.exe contains a nontrivial amount of code and inevitably a bunch of it calls the malloc family. Linking to mozglue would violate the constraint at https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/WindowsCrtPatch.h#31-32.

Without a pressing need to install the blocklist super early, I'm not inclined to spend time dealing with the library relationships. I propose that we let xul.dll do the initialize call from XRE_InitChildProcess.
(Assignee)

Comment 5

3 years ago
Created attachment 8565269 [details] [diff] [review]
Part 1: Use the DLL blocklist in plugin-container processes

I tested that our asserts are still happy on XPSP2.
Attachment #8565269 - Flags: review?(benjamin)
(Assignee)

Comment 6

3 years ago
Created attachment 8565270 [details] [diff] [review]
Part 2 - Block rndlnpshimswf.dll and rndlmainbrowserrecordplugin.dll

I checked that these DLLs don't get loaded, and that the Real addon gracefully handles the failed loads.
Attachment #8565270 - Flags: review?(benjamin)
(Assignee)

Comment 7

3 years ago
Created attachment 8565272 [details] [diff] [review]
Part 2 - Block rndlnpshimswf.dll and rndlmainbrowserrecordplugin.dll

I checked that these DLLs don't get loaded, and that the Real addon gracefully handles the failed loads.
Attachment #8565270 - Attachment is obsolete: true
Attachment #8565270 - Flags: review?(benjamin)
Attachment #8565272 - Flags: review?(benjamin)
(Reporter)

Updated

3 years ago
Attachment #8565269 - Flags: review?(benjamin) → review+
(Reporter)

Comment 8

3 years ago
Comment on attachment 8565272 [details] [diff] [review]
Part 2 - Block rndlnpshimswf.dll and rndlmainbrowserrecordplugin.dll

I think we should block all versions.
Attachment #8565272 - Flags: review?(benjamin) → review-
(Reporter)

Comment 9

3 years ago
And just to avoid friction, r+ with ALL_VERSIONS.
(Assignee)

Comment 11

3 years ago
Created attachment 8565757 [details] [diff] [review]
Part 2 - Block rndlnpshimswf.dll and rndlmainbrowserrecordplugin.dll

as landed, with ALL_VERSIONS
Attachment #8565272 - Attachment is obsolete: true
Attachment #8565757 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e873eb61dba8
https://hg.mozilla.org/mozilla-central/rev/90995b26caef
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
We've never release noted blocking DLLs before - is there something special about these two (very long-named) that makes us want to change that?  Should I be noting that the Real addon will be blocked from this version forward (which is also something we don't typically put in release notes).
Flags: needinfo?(dmajor)
Flags: needinfo?(benjamin)
(Assignee)

Comment 14

3 years ago
The RealDownloader will be blocked. Somebody may be deliberately using it, and may be surprised when it stops working. (Or maybe not, I don't know.) If these things are normally not mentioned, then ok.
Flags: needinfo?(dmajor)
Ok, good to know.  As addon blocking is not something we release note, I'm removing the flag here.  NI? to Tyler so that UA has awareness that this may come up in feedback.
relnote-firefox: ? → ---
Flags: needinfo?(benjamin) → needinfo?(tdowner)

Updated

2 years ago
Flags: needinfo?(tdowner)
Product: addons.mozilla.org → Toolkit
You need to log in before you can comment on or make changes to this bug.