Closed Bug 1286368 Opened 8 years ago Closed 8 years ago

Block old versions of McAfee SiteAdvisor (pre 4.0.0)

Categories

(Toolkit :: Blocklist Policy Requests, defect)

48 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: philipp, Assigned: jorgev)

References

Details

(Whiteboard: [extension])

Attachments

(1 file)

Extension name: McAfee SiteAdvisor 
Extension UUID: {4ED1F68A-5463-4931-9384-8FFF5ED91D92}
Extension versions to block: < 4.0.0
Applications, versions, and platforms affected: Firefox 48 and later on Windows
Block severity: tbd

Homepage, AMO listing, other references and contact info: www.siteadvisor.com

Reasons: old versions of McAfee SiteAdvisor cause an increase in startup crashes starting with firefox 48.0b, see https://bugzilla.mozilla.org/show_bug.cgi?id=1131180#c8
though its volume has decreased over the cycle but this will probably just mean that affected users gave up on firefox: https://crash-stats.mozilla.com/signature/?product=Firefox&release_channel=beta&date=>%3D2016-05-20&signature=je_free | js%3A%3Actypes%3A%3ACData%3A%3AFinalize#graphs

addon correlation data for 48.0b1 from 2016-06-16:
  je_free | js::ctypes::CData::Finalize|EXCEPTION_ACCESS_VIOLATION_READ (106 crashes)
     99% (105/106) vs.   1% (307/23073) {4ED1F68A-5463-4931-9384-8FFF5ED91D92}
          4% (4/106) vs.   0% (4/23073) 3.6.5
          9% (10/106) vs.   0% (11/23073) 3.6.6
         44% (47/106) vs.   0% (47/23073) 3.7.0
         27% (29/106) vs.   0% (29/23073) 3.7.1
         14% (15/106) vs.   0% (16/23073) 3.7.2
          0% (0/106) vs.   0% (1/23073) 4.0.16
          0% (0/106) vs.   0% (34/23073) 4.0.20
          0% (0/106) vs.   0% (1/23073) 4.0.6
          0% (0/106) vs.   1% (161/23073) 5.0.169.0
          0% (0/106) vs.   0% (3/23073) 5.0.218.0
They've fixed the issue in version 5.0.226.0, which they are releasing now.
the correlation data would suggest that version 4.0 wasn't causing this particular signature anymore either. not sure if other crashes should taken into consideration & the blocking range extended as well...
I'm not familiar with correlation data - is less than 4.0.0 (4.0.16?) still sufficient/appropriate?
What about blocking that for now and, after McAfee gets back to us, block the other affected versions?
yes, that sounds good. 
unfortunately i don't know what their highest released version number for the 3.x branch is, so in case it isn't possible with the blocklist to target anything below 4.0.0, maybe set 3.9.99 as the highest affected version just to be on the safe side...
It'd be good to get some data on the usage of those versions. We need to be careful about blocking potentially millions of users.
i could reproduce the issue with 48.0b7 when installing siteadvisor 3.7.0 - firefox will just crash a few seconds after being launched repeatedly (but i was also automatically upgraded to 4.0.20 after i rebooted my system once and the issue ceased). 
if there were a substantial amount of users of this addon on 3.x it becomes even more important to blocklist, because those users would likely end up in troubles after the 48 release...
Has there been any contact with McAfee representatives?  They'll likely know how many of their users have upgraded already.
philipp, can you reproduce bug 1277897 as well? 

(In reply to Andrew Williamson [:eviljeff] from comment #9)
> Has there been any contact with McAfee representatives?  They'll likely know
> how many of their users have upgraded already.

The issue that they have found has been fixed in 5.0.226.0 and they've rolled out the update to 15% of their users.
Flags: needinfo?(madperson)
(In reply to Marco Castelluccio [:marco] from comment #10)
> philipp, can you reproduce bug 1277897 as well? 
no, unfortunately not. a common theme in the user comments for that signature seems to be logging into their gmail account, but that did work fine for me. 

today's correlation data for the npmcffplg32.dll@0x5a3d signature on 47.0 shows that in general these crashes occur less frequently (like for one out of 100 siteadvisor/webadvisor users) but still with version 5.0.226.0 too, whereas je_free | js::ctypes::CData::Finalize on 48.0b happens for each of the siteadvisor users on 3.x which is recorded in our crash stats data.
Flags: needinfo?(madperson)
Assignee: nobody → jorge
(In reply to Marco Castelluccio [:marco] from comment #10)
> The issue that they have found has been fixed in 5.0.226.0 and they've
> rolled out the update to 15% of their users.

We should deploy this block when the majority of their users have been updated, to avoid impacting millions of users. Can they let us know when they've done that?
i'm not sure if i understand how the update from siteadvisor 5.0.218.0 to 5.0.226.0 plays into consideration here... 
the requested blocklisting should only target siteadvisor 3.x on firefox 48 and above (due to the startup crashes occuring with this combination - bug 1131180). mcafee have released their siteadvisor branches 4.x and 5.x already a while ago, so only a fraction of their users should be affected by blocking version 3.x now.
we're interested in the % of users on the versions being blocklisted
the only insight on that available to me would be addon correlation data from crash stats:
out of yesterday's 1581 recorded crashes from users with the siteadvisor extension installed on firefox 47.0 eight (0.5%) were on version 3.x, 181 (11.5%) were on 4.x and 1392 (88%) on 5.x
The block is now live: https://addons.mozilla.org/en-US/firefox/blocked/i1245
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
(In reply to Jorge Villalobos [:jorgev] from comment #16)
> The block is now live: https://addons.mozilla.org/en-US/firefox/blocked/i1245

I don't see it in the list at https://addons.mozilla.org/en-US/firefox/blocked/. Does that list get auto-updated when new blocks are added?
It should appear now. I think this is related to code that groups blocks for the same add-on (there is an older block for SiteAdvisor). Even before this change I just made the block was present in the downloaded blocklist.
> It should appear now. 

It does, thank you.
Could you verify that the block is in place? We're still seeing crashes with versions of the addon < 4.0.0 (e.g. 3.7.0, 3.7.1, 3.7.2, 3.6.6, 3.6.5).
Flags: needinfo?(jorge)
Flags: needinfo?(awilliamson)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I can see this entry in the blocklist.xml file in my profile folder:

<emItem blockID="i1245" id="{4ED1F68A-5463-4931-9384-8FFF5ED91D92}">
  <versionRange minVersion="0" maxVersion="3.9.9" severity="1">
  </versionRange>
  <prefs>
  </prefs>
</emItem>

Perhaps affected users are not even able to download the blocklist?
(In reply to Marco Castelluccio [:marco] (PTO until August 9) from comment #21)
> Perhaps affected users are not even able to download the blocklist?

We can't really know without a reproducible case. Have the crashes reduced or are they essentially the same?
Flags: needinfo?(mcastelluccio)
Flags: needinfo?(jorge)
Flags: needinfo?(awilliamson)
(In reply to Jorge Villalobos [:jorgev] from comment #22)
> We can't really know without a reproducible case. Have the crashes reduced
> or are they essentially the same?

the crashes haven't really gone down on 48 beta after the block went live in comment #16 : http://bit.ly/2aRqQY3 - the addon is still showing up as active in those crash reports...
 
maybe we can change the maxVersion of the blocklist entry for the extensions to the known highest version of the 3.x branch (3.7.2 ?), change the block severity to a hard block and really target only firefox 48.0 and higher (in order to avoid that users re-enable the addon and maneuver themselves into a permanent startup crash without knowing it) to see if this is making a difference in crash stats?

also, would the updated blocklist.xml be baked directly into a newly released version of firefox (48.0.1), so it reaches affected users for sure and they don't depend on the downloadable blocklist?
Flags: needinfo?(mcastelluccio)
(In reply to Jorge Villalobos [:jorgev] from comment #22)
> We can't really know without a reproducible case. Have the crashes reduced
> or are they essentially the same?

Is this the first time that an addon causing a startup crash has been blocklisted?
What happened in the past?

I'm not familiar with how the blocklist works, when is it downloaded? The crash is
happening within 60 seconds from when the browser is launched.

Since the addon comes with a DLL injected in the Firefox process, can we block the
addon by the DLL version? (The crash is not caused by the DLL, so we can't simply block
the DLL). I believe the DLL is not part of the addon, but we can detect the addon version
by checking the DLL version. This way we could block the addon without relying on the
downloadable blocklist.

(In reply to [:philipp] from comment #23)
> also, would the updated blocklist.xml be baked directly into a newly
> released version of firefox (48.0.1), so it reaches affected users for sure
> and they don't depend on the downloadable blocklist?

Looks like the blocklist.xml file is already baked in the Firefox package downloaded
from https://www.mozilla.org/en-US/firefox/all/#it (at least for Linux), and it does
contains the rule from comment 21.
What happens when there's an update? Is a new blocklist file downloaded as well as part
of the update?
If it is, why are users still crashing and have the addon enabled, given that it is in the
blocklist?
Flags: needinfo?(jorge)
Can you help answering the questions from comment 24? Do we update the blocklist.xml file when we update Firefox?
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(mhowell)
Marco, app update will update all files that it is given to update. As long as the blocklist is present when the update mar file is generated it will be included in the update.
Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(mhowell)
The blocklist can update itself separately from Firefox. It's a completely separate system from application update, so I don't really know how it works and I couldn't tell you whether that update can happen before this crash hits or not; I took a quick look at the blocklist service code [1], and I can see the code that runs that update, but it isn't clear to me how it's triggered.

[1] https://dxr.mozilla.org/mozilla-central/rev/6cf0089510fad8deb866136f5b92bbced9498447/toolkit/mozapps/extensions/nsBlocklistService.js
The blocklist update is triggered by a timer and the blocklist located in the profile is the one that is updated by this method. The blocklist has changed significantly since I first wrote it many years ago and mossop understands it much better than I do.
What I'd like to understand is whether the blocklist.xml file is included in the update and, if it is, whether it can replace an outdated blocklist.xml file downloaded through the blocklist service (since they're crashing at startup, the blocklist service is likely unable to update the blocklist).
If that's the case, we can conclude that all users with Firefox 48 have the updated blocklist but, since they're still crashing and still have the addon that we have supposedly blocklisted, the blocklist is ineffective.
Flags: needinfo?(dtownsend)
(In reply to Marco Castelluccio [:marco] from comment #29)
> What I'd like to understand is whether the blocklist.xml file is included in
> the update and, if it is, whether it can replace an outdated blocklist.xml
> file downloaded through the blocklist service (since they're crashing at
> startup, the blocklist service is likely unable to update the blocklist).
> If that's the case, we can conclude that all users with Firefox 48 have the
> updated blocklist but, since they're still crashing and still have the addon
> that we have supposedly blocklisted, the blocklist is ineffective.

Yes, we include updated blocklists in Firefox updates and they replace the one in the profile assuming it has newer data.
Flags: needinfo?(dtownsend)
i have played around with installing siteadvisor 3.7.0 again and there seem to be two issues with addon signing and the blocklist that play a role here.

* in versions below 48.0 siteadvisor 3.7.0 is force-disabled in the addons manager due to the extension not being signed, but after the 48.0 update siteadvisor 3.7.0 appears in about:addons enabled and as if it was fully signed. this probably explains the rise of the [@ je_free | js::ctypes::CData::Finalize] crash in version 48.0 (when changing xpinstall.signatures.required to false, i could also get firefox 47.0 to crash persistently though)

* the blocklist entry is filtering through to my installation (the addon is labelled as unstable in the addons manager) but that doesn't seem to disable the addon by default in all instances. i could reproduce the issue like this with a fresh profile trying to mimic what an affected user would have gone through as well:
- install firefox 30
- install siteadvisor 3.7.0 with the default parameters while being offline (otherwise the installer will install webadvisor 5) 
- open firefox and "allow this installation" in the full screen dialog "another program would like to modify firefox with the following addon: siteadvisor 3.7.0"
- update firefox in the "about firefox panel" to the 43.0.1 watershed. after a restart the siteadvisor addon will show up in about:addons as disabled because it isn't signed.
- force a blocklist ping so that the blocklist entry discussed in this bug is applied to the profile
- update firefox in the "about firefox panel" to version 48.0. after a restart the siteadvisor addon will show up in about:addons as known to cause stability issues but fully enabled (even when i manually edited the entry to severity="3" in the blocklist.xml file in the profile, the addon would stay enabled).
(In reply to [:philipp] from comment #31)
> i have played around with installing siteadvisor 3.7.0 again and there seem
> to be two issues with addon signing and the blocklist that play a role here.
> 
> * in versions below 48.0 siteadvisor 3.7.0 is force-disabled in the addons
> manager due to the extension not being signed, but after the 48.0 update
> siteadvisor 3.7.0 appears in about:addons enabled and as if it was fully
> signed. this probably explains the rise of the [@ je_free |
> js::ctypes::CData::Finalize] crash in version 48.0 (when changing
> xpinstall.signatures.required to false, i could also get firefox 47.0 to
> crash persistently though)

This is worrying, I'm going to try to set up a VM so I can test this out today.

> * the blocklist entry is filtering through to my installation (the addon is
> labelled as unstable in the addons manager) but that doesn't seem to disable
> the addon by default in all instances. i could reproduce the issue like this
> with a fresh profile trying to mimic what an affected user would have gone
> through as well:
> - install firefox 30
> - install siteadvisor 3.7.0 with the default parameters while being offline
> (otherwise the installer will install webadvisor 5) 
> - open firefox and "allow this installation" in the full screen dialog
> "another program would like to modify firefox with the following addon:
> siteadvisor 3.7.0"
> - update firefox in the "about firefox panel" to the 43.0.1 watershed. after
> a restart the siteadvisor addon will show up in about:addons as disabled
> because it isn't signed.
> - force a blocklist ping so that the blocklist entry discussed in this bug
> is applied to the profile
> - update firefox in the "about firefox panel" to version 48.0. after a
> restart the siteadvisor addon will show up in about:addons as known to cause
> stability issues but fully enabled (even when i manually edited the entry to
> severity="3" in the blocklist.xml file in the profile, the addon would stay
> enabled).

This is down to a bug in how the blocklist applies the softblocked attribute to add-ons. I've filed it as bug 1294439.
Depends on: 1294439
(In reply to Dave Townsend [:mossop] from comment #33)
> This is worrying, I'm going to try to set up a VM so I can test this out
> today.

i did some manual regression testing with builds from mozilla-central and 20160401030216 was the first 48.0a1 build exhibiting that behaviour. from the pushlog https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=494289c72ba3997183e7b5beaca3e0447ecaf96d&tochange=538d248fa252a4100082fd9bc3fdc08d322cda22 it's probably bug 1255590 causing this - the siteadvisor addon is getting inserted into firefox via the registry...
(In reply to Dave Townsend [:mossop] from comment #33)
> (In reply to [:philipp] from comment #31)
> > i have played around with installing siteadvisor 3.7.0 again and there seem
> > to be two issues with addon signing and the blocklist that play a role here.
> > 
> > * in versions below 48.0 siteadvisor 3.7.0 is force-disabled in the addons
> > manager due to the extension not being signed, but after the 48.0 update
> > siteadvisor 3.7.0 appears in about:addons enabled and as if it was fully
> > signed. this probably explains the rise of the [@ je_free |
> > js::ctypes::CData::Finalize] crash in version 48.0 (when changing
> > xpinstall.signatures.required to false, i could also get firefox 47.0 to
> > crash persistently though)
> 
> This is worrying, I'm going to try to set up a VM so I can test this out
> today.

I've filed bug 1294483 for this.
(In reply to Marco Castelluccio [:marco] from comment #24)
> Is this the first time that an addon causing a startup crash has been
> blocklisted?
> What happened in the past?

It's not the first time. Users experiencing a persistent startup crash are generally on their own, but blocking an add-on due to a startup crash at least prevents more users to experience the crash.

> Since the addon comes with a DLL injected in the Firefox process, can we
> block the
> addon by the DLL version?

No, only by add-on version.
Flags: needinfo?(jorge)
since there was nothing wrong with the blocklist entry itself, i will set the status of the bug to resolved again.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: