525103 - Generate list of DLLs to Blocklist

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Damon Sicore (:damons)]

Need the list of DLLs to blocklist for bug 524904.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

As a first put, the list used by chrome is here:

http://www.google.com/codesearch/p?hl=en&sa=N&cd=4&ct=rc#YwaaF8DorRY/src/chrome/browser/sandbox_policy.cc&exact_package=git://github.com/kesor/chromium.git&l=23

Comment 2

[Mike Shaver (:shaver emeritus)]

Yeah, we can't use that out of the box because it blocks things that we use as plugins, I do believe, but it's worth asking them about.

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

plugins that crash us?

Comment 4

[chris hofmann]

Attachment: hitlist of .dll's found in crash data compared to the chromium sandbox

I banged http://www.google.com/codesearch/p?hl=en&sa=N&cd=4&ct=rc#YwaaF8DorRY/src/chrome/browser/sandbox_policy.cc&exact_package=git://github.com/kesor/chromium.git&l=23 

against dbarons list from http://people.mozilla.com/crash_analysis/20091102/20091102_Firefox_3.0.14-interesting-modules-with-versions.txt.bz2

and got the resulting attachment of .dll' we might be likely to be finding and blocking.   I guess highest priority might be to test these with higher probability to appear in the firefox process space

Comment 5

[Damon Sicore (:damons)]

Where are we taking submissions?  If here, I propose:

BtwVdpCapFilter.dll

from:

http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.5.4&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=BtwVdpCapFilter.dll%400xa345

Comment 6

[Johnathan Nightingale [:johnath]]

I think we need a policy here, before we can figure out what gets added.

The plugin blocklist policy (https://wiki.mozilla.org/Blocklisting) says:

* Block add-ons & plugins:
** with versions with known vulnerabilities or major user-facing issues
** with version ranges with known vulnerabilities
** with fatal bugs (client crashes on startup or something causing an endless loop of unusability)

* Don't block:
** before we work with author/vendor to send out an update, so don't block the most recent version of a major addon/plugin
** for minor bugs or non-popular addons/plugins (crashes on event calls, messed up UI, etc.)
** if user has disabled compatibility checking and add-on causes problems/crashes
** plugins that are pre-release, alpha or beta

Most of this carries over nicely, but we should keep in mind that the DLL blocklist is a slower-moving thing. We don't ping every day for this, it's a file that's delivered with install and minor updates. It also doesn't allow for soft-blocking, which plugin blocklisting does - in fact there is no UI even indicating that it's been disabled.

As a first pass then, I propose:

We block DLLs that:
 - Hurt users (crashing, data loss, performance hits, &c) AND
 - Cannot be useful to their users (because they are incompatible with the versions attempting to load them, because they crash frequently, or crash at startup, &c), AND
 - Cannot be fixed in a timely fashion by their authors (because the author can't be contacted, because the author isn't responding in a timely way, because the author is malicious, &c), AND
 - Cannot be mitigated through a more user-controlled mechanism (e.g. the addon/plugin blocklist)

By that math, then, things like NPFFAddOn.dll are an easy pass (malware, crashy). BtwVdpCapFilter.dll is fuzzier for me - it's clearly crashy, and while I don't have it in front of me, the internet seems to think it's something to do with bluetooth video cams - does that mean it might actually have a vendor?

I'd like to get the policy language to a place where we're happy with it before considering individual nominations. A bug is probably the wrong place for that, though; I'll take it to dev.platform/planning.

Comment 7

[u88484]

Let me see if I get this right with an example.  If GoogleDesktopNetwork3.dll, as stated in bug 519344, was blocked and google fixed the crash in said dll...someone at Mozilla would have to manually remove the .dll from the blocklist?  Or Google would have to rename the .dll?

Comment 8

[Johnathan Nightingale [:johnath]]

(In reply to comment #7)
> Let me see if I get this right with an example.  If GoogleDesktopNetwork3.dll,
> as stated in bug 519344, was blocked and google fixed the crash in said
> dll...someone at Mozilla would have to manually remove the .dll from the
> blocklist?  Or Google would have to rename the .dll?

Correct, though I'm not sure what you mean by "manually" - every code change involves some person actually doing it, yes?

Comment 9

[u88484]

(In reply to comment #8)
> (In reply to comment #7)
> > Let me see if I get this right with an example.  If GoogleDesktopNetwork3.dll,
> > as stated in bug 519344, was blocked and google fixed the crash in said
> > dll...someone at Mozilla would have to manually remove the .dll from the
> > blocklist?  Or Google would have to rename the .dll?
> Correct, though I'm not sure what you mean by "manually" - every code change
> involves some person actually doing it, yes?

If that .dll was blocked then Google would have to somehow figure out that Mozilla has blocked that dll since it crashes, figure out why it's crashing, fix said crash, rename the dll file itself and wherever else the file name was used, release new version of software.

What I'm trying to get at is how the heck our legitimate companies/people supposed to know that Mozilla has blocked a certain dll and that is why users are complaining that their software no longer works?  And that they now have to figure out a new name for the dll and rename it everywhere in their code.

Comment 10

[timeless]

i thought we said we could blocklist dll-version pairs, if so, we'd just blocklist the googledesktop-version pairs. and all they'd have to do is bump the version field, which they should do anyway.

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Yep, we can block a dll name and maximum version, so someone who fixes a bug could just bump their version number, assuming that we put in a versioned block.  We might not, for example, if we were unable to contact them to figure out what the correct maximum version should be.  We have the version info in crash-stats, so we should be able to make a good guess at this as necessary.

The blocklist information for DLLs should be visible in the same space as the rest, at http://www.mozilla.com/blocklist/ .

Comment 12

[u88484]

Thanks Vlad.  That makes me feel a lot better about this bug that also the version number will be blocked along with the name.

Comment 13

[Johnathan Nightingale [:johnath]]

Thanks for the intersection list, chofmann. One of the catches with dbaron's list is that a single user who has the DLL loaded could potentially be 8% of crashes, which skews the scoring. Nevertheless, this points out a couple of serious offenders.

I'd like to keep the initial list small - adding to it is straightforward, but starting with a huge list threatens to block us on examining each specific case.  With that in mind, then, I'd propose this initial list:

avgrsstx.dll // AVG 8
r3hook.dll // Kaspersky
sahook.dll // McAfee Site Advisor
smumhook.dll // Spyware Doctor version 5 and above

Each of these appears in Google's list, and appears prominently in some of our crash signatures.

I think NPFFAddon.dll (bug 519343) is an obvious candidate as well, but can probably be handled in its own bug.

As for Damon's question in comment 5, I think we should be filing new bugs for each new addition. We don't have a component for these yet, and we probably don't want to complicate the existing blocklisting component right at the moment. I don't want to saddle this with too much process - how's about for now we file new bugs which block this one, as a way to track dependencies. And then, once we have hit our stride on the process, we'll create a bugzilla component for things, hmm?

If no one has any show stopping objections, my next step will be to try to reach each of these DLL authors to find out how they feel about being blocked, as well as trying to nail down DLL versions for each entry.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Johnathan, another candidate which crashes Firefox directly on start-up is GoogleDesktopMozilla.dll for version 4.2006.509.1244. Later versions don't show this crash.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #13)
> sahook.dll // McAfee Site Advisor

See bug 521745; I think the problems caused by SiteAdvisor should be fixed now, although they might not have gotten the update to all users.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #15)
> (In reply to comment #13)
> > sahook.dll // McAfee Site Advisor
> 
> See bug 521745; I think the problems caused by SiteAdvisor should be fixed now,
> although they might not have gotten the update to all users.

... but their update uptake rate seems to have been pretty good last I checked (which was a few weeks ago).

I don't think SiteAdvisor fits the DLL blocklist policy given that we could use the addon blocklist on it.  That said, we could consider using DLL blocklist for SiteAdvisor if we can get better version granularity, since they didn't bump their extension version when they fixed the problem.

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Would the blocklist also be able to use minimum versions for checking?

Because with bug 527540, I think it only crashes with the most (more?) recent version(s?) of nvLsp.dll.

Comment 18

[Kev Needham [:kev]]

Outreach started per the list in Comment 13. Note that AVG 8 is already blocklisted (bug 479095), so if we are continuing to see crashes from it, we may want to adjust the entry.

Comment 19

[Johnathan Nightingale [:johnath]]

Attachment: Very short initial list, to get things moving

Okay, here's my concern: 

- in the last couple days I've heard Tony say he needs some entries in the blocklist in order to be able to effectively QA things, AND
- I've heard people like Damon say "where are we on bug 525103 because I have other DLLs I'd like to add" or words to that effect, effectively saying that other DLL blocking is waiting on this bug to be resolved.

There's nothing special about this bug over other requests to block, except the fact that this one involves the first entries. Each request to add entries to the list should be basically independent of every other such request - but right now they're blocking on this one getting the ball rolling.

With that in mind then, I think we should close this bug ASAP - it's creating a process bottleneck that I don't think should exist. The patch attached blocks AVG8 (the plugin's already blocklisted, so I guess these DLLs are being loaded some other way?) and NPFFAddon (see bug 519343).

I've assumed that Socorro's reported version numbers are decimal, not hex - someone check me on that?

I'll file follow ups on the other DLLs mentioned above, so that they can be handled independently, which is also what I hope others will do. The patch format is easy as pie, it's really just about making sure we get the version numbers right, and inform vendors where applicable.

Comment 20

[chris hofmann]

(In reply to comment #19)
> Created an attachment (id=413328) [details]
> Very short initial list, to get things moving
> 
> Okay, here's my concern: 
> 
> - in the last couple days I've heard Tony say he needs some entries in the
> blocklist in order to be able to effectively QA things, AND
> - I've heard people like Damon say "where are we on bug 525103 because I have
> other DLLs I'd like to add" or words to that effect, effectively saying that
> other DLL blocking is waiting on this bug to be resolved.
> 
> There's nothing special about this bug over other requests to block, except the
> fact that this one involves the first entries. Each request to add entries to
> the list should be basically independent of every other such request - but
> right now they're blocking on this one getting the ball rolling.
> 

do we also need a beta to test the implementation *and* impact of actually having a populated list?  testing that as part of an RC1 seems risky if the RC test cycle is short.

should we track that in this bug, or some other?

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Just one more note. Beside the hard blocking of modules we also have the components.list feature to test. We don't have updates yet which we can test until bug 528457 is fixed. 

(In reply to comment #19)
> - in the last couple days I've heard Tony say he needs some entries in the
> blocklist in order to be able to effectively QA things, AND

Meanwhile I have created try server builds and they are available in our test plan. Those hard block npFFAddon.dll and GoogleDesktopMozilla.dll for a given and all versions.

Comment 22

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 413328 [details] [diff] [review]
Very short initial list, to get things moving


>+// Convert the 4 (decimal) components of a DLL version number into a
>+// single unsigned long long, as needed by the blocklist
>+#define VERSION_FROM_COMPONENTS(a,b,c,d)\
>+  (a##ULL << 48) + (b##ULL << 32) + (c##ULL << 16) + d##ULL

Looks fine, but please call this a less-unwieldy MAKE_VERSION() and put parens around the whole thing.. that is:

#define MAKE_VERSION(a,b,c,d) \
  ((a##ULL << 48) + (b##ULL << 32) + ... )

Comment 23

[Johnathan Nightingale [:johnath]]

Attachment: AVG+NPFFAddon, with review addressed

Comment 24

[Johnathan Nightingale [:johnath]]

Hoping to land this on trunk today, but flagging just in case.

Comment 25

[Johnathan Nightingale [:johnath]]

http://hg.mozilla.org/mozilla-central/rev/8652dc7f3840

Comment 26

[Johnathan Nightingale [:johnath]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/74dfc0468e0a

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Tony, could you please verify the npffAddon.dll entry on a native Windows machine? I don't have one and cannot install that malware because it blocks itself in VMs. Thanks.

Comment 28

[Tony Chung [:tchung]]

(In reply to comment #27)
> Tony, could you please verify the npffAddon.dll entry on a native Windows
> machine? I don't have one and cannot install that malware because it blocks
> itself in VMs. Thanks.

Fwiw, i verified npffAddOn.dll gets blocklisted against a win XP and Win7 64-bit boxes.   Those two machines have long been wiped and restored back to good health now.

Comment 29

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Tony! Marking bug as verified1.9.2.