Generate list of DLLs to Blocklist

RESOLVED FIXED in Firefox 3.7a1

Status

()

RESOLVED FIXED
10 years ago
8 years ago

People

(Reporter: damons, Assigned: johnath)

Tracking

(Blocks: 1 bug, {verified1.9.2})

3.6 Branch
Firefox 3.7a1
verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3.6 +

Firefox Tracking Flags

(status1.9.2 beta4-fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Need the list of DLLs to blocklist for bug 524904.
Flags: blocking-firefox3.6+
(Assignee)

Updated

10 years ago
Assignee: nobody → johnath
(Assignee)

Updated

10 years ago
OS: Mac OS X → All
Hardware: x86 → All
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 4

10 years ago
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
(Assignee)

Comment 6

10 years ago
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

10 years ago
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?
(Assignee)

Comment 8

10 years ago
(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

10 years ago
(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

10 years ago
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.
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/ .

Updated

10 years ago
Blocks: 527543

Comment 12

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

Comment 13

10 years ago
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.
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.
(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.
(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.
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.
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.
Whiteboard: [needs patch]
(Assignee)

Comment 19

10 years ago
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.
Attachment #413328 - Flags: review?(vladimir)
(Assignee)

Updated

10 years ago
Whiteboard: [needs patch] → [needs review vlad]

Comment 20

10 years ago
(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?
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 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) + ... )
Attachment #413328 - Flags: review?(vladimir) → review+
(Assignee)

Comment 23

10 years ago
Attachment #413328 - Attachment is obsolete: true
(Assignee)

Comment 24

10 years ago
Hoping to land this on trunk today, but flagging just in case.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Whiteboard: [needs review vlad] → [has patch]
Whiteboard: [has patch] → [can land]
(Assignee)

Comment 25

10 years ago
http://hg.mozilla.org/mozilla-central/rev/8652dc7f3840
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [can land] → [needs-192-checkin]
(Assignee)

Updated

9 years ago
status1.9.2: --- → final-fixed
Whiteboard: [needs-192-checkin]
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.
Target Milestone: --- → Firefox 3.7a1
Version: unspecified → 3.6 Branch
(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.
Thanks Tony! Marking bug as verified1.9.2.
Keywords: verified1.9.2
You need to log in before you can comment on or make changes to this bug.