Closed Bug 838996 Opened 11 years ago Closed 11 years ago

AVG Security Toolbar 14.0.3.14 disables all content policies

Categories

(Toolkit :: Blocklist Policy Requests, defect)

Product:
Toolkit
Component:
Blocklist Policy Requests
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jwkbugzilla, Unassigned)

References

(
URL
)

Details

(Keywords: sec-high) 

We got many users reporting that AVG Security Toolbar 14.0.3.14 breaks Adblock Plus. Upon closer investigation, the issue isn't limited to Adblock Plus - the newest version of AVG Security Toolbar rather removes every single content policy, including the ones implementing security mechanisms in the browser. This version isn't available from the website yet and the free antivirus seems to be still using the previous version. So judging by the report, at the moment only users of AVG Internet Security 2013 are affected. The link http://toolbar.avg.com/partners/avg/14.0.3.14/download/avg-ie.xpi works to get the extension.

The issue is caused by the following function in components/avg-dnt-policy.js:

    disableDNT: function(state){
        var categoryManager = Cc["@mozilla.org/categorymanager;1"]
                      .getService(Components.interfaces.nsICategoryManager);
		      
        if(state){
            categoryManager.deleteCategory("content-policy");
            categoryManager.deleteCategory("net-channel-event-sinks");
        }else{
            categoryManager.addCategoryEntry("content-policy", "@mozilla.org/content-policy;1", "@mozilla.org/content-policy;1", false, true);
            categoryManager.addCategoryEntry("net-channel-event-sinks", "@mozilla.org/content-policy;1", "@mozilla.org/content-policy;1", false, true);
            this.LoadConfiguration();
        }
    },

DNT seems to be disabled by default so this component calls disableDNT(true) on startup. And instead of removing itself from the category it deletes the entire category (and "net-channel-event-sinks" category as well). Obvious solution:

        if(state){
            categoryManager.deleteCategoryEntry("content-policy", "@mozilla.org/content-policy;1", false);
            categoryManager.deleteCategoryEntry("net-channel-event-sinks", "@mozilla.org/content-policy;1", false);
        }else{

I'll see whether I can find somebody to contact at AVG. And I am wondering whether causing this kind of breakage warrants blacklisting...
PS: Yes, AVG Security Toolbar actually uses "@mozilla.org/content-policy;1" as contract ID of its component. And by a lucky chance it doesn't even conflict with any of Mozilla's contract IDs.
Severity: normal → critical
Can we get the addon blocklisted ASAP. As far as I see, the addon essentially causes random security
problems, though it is not clear how serious ones.
Or do we have contacts to get them to fix this on their side ASAP.
Note: The version currently available from http://www.avg.com/secure-search (AVG Security Toolbar 14.0.0.12) contains exactly the same code to register/unregister the DNT component. The only difference is the defaults - it doesn't have DNT disabled by default (see STARTUP_DISABLED_DNT definition in modules/avg-dnt-adapter.js) so disableDNT(true) doesn't run.

Also, I see now that they are using a custom update mechanism, checking the URL http://stats.avg.com/services/toolbar_updater.aspx?data=%3C?xml%20version=%221.0%22%20encoding=%22UTF-8%22?%3E%3CToolbar%3E%3Cversion%3E14.0.3.14%3C/version%3E%3Cbrowser%3EFF%3C/browser%3E%3CffPackage%3Eie%3C/ffPackage%3E%3Cpid%3Eavg%3C/pid%3E%3Ccid%3E{abf135de-8437-41a8-a9ac-0bd3978fa6a9}%3C/cid%3E%3Cmid%3E%3C/mid%3E%3Cds%3E%3C/ds%3E%3Clang%3E%3C/lang%3E%3Cpr%3E%3C/pr%3E%3Cd%3E2013-02-7%2011%3A16%3A52%3C/d%3E%3Cigtbinitialstatus%3Eunknown%3C/igtbinitialstatus%3E%3Cigtbstatus%3Eunknown%3C/igtbstatus%3E%3CadditionalInfo%3Eapp-profile%3C/additionalInfo%3E%3C/Toolbar%3E (yes, via plain HTTP and no further protection from what I can tell). At the moment this serves up version 13.2.0.5 so my assumption about users of the free antivirus being unaffected should be still true - but maybe the parameters (and the result) are different there.

(In reply to Olli Pettay [:smaug] from comment #2)
> Can we get the addon blocklisted ASAP. As far as I see, the addon
> essentially causes random security
> problems, though it is not clear how serious ones.

From what I can tell, it disables CSP, prevention of inline JS execution via <frame src="javascript:...">, mixed content blocking, built-in content blocker, disabling of JavaScript/frames/images via preferences, prevention of requests initiated from data: documents - and that's only the built-in content policies.
(In reply to Olli Pettay [:smaug] from comment #2)
> Or do we have contacts to get them to fix this on their side ASAP.

Looks like we were able to reach someone from AVG on our side. I just hope that I articulated the severity of this issue well enough.
Group: core-security → addons-security
Component: General → Add-on Security
Product: Firefox → addons.mozilla.org
Version: Trunk → unspecified
Wladimir,

Can you share their response? Are they planning to take action? Also, when you say "our side" are you referring to adblockplus or Mozilla?

Thanks
This is somewhat odd. AMO has an extension called "AVG Do Not Track" that's at version "1.0.0.2" released january 16, an update to 1.0.0.1 released in November. This addon looks sane with regard to this issue:

+ The component name is "@avg.com/content-policy;1", not @mozilla.org
+ category registration is standard
+ there's no explicit update mechanism for the code, it uses
  the standard AMO update. (It does update data files telling it
  about new things to block.)

I could easily believe they had old code with problems that got cleaned up through the AMO review process, and indeed they had an initial rejection that mentioned the @mozilla.com ID issue among other things. There was nothing about the category nuking issue so they must have gotten that right from the start.

Wouldn't surprise me if the Toolbar had an old version of the Do Not Track addons, but I am a little puzzled that the old DNT code is apparently showing up in NEW versions of the toolbar.

Another oddity: the statistics pages show only 11,000 downloads total, but show a user-count in the millions. Most of those users are using a version "12.0.0.2189" which we've never hosted, but sound like the AVG toolbar version scheme mentioned above. Maybe they reused the ID? That will make eventual updates interesting...

https://addons.mozilla.org/en-US/firefox/addon/avg-do-not-track/statistics/usage/versions/?last=365
On SUMO I found a user who had the 12.0.0.2166 version of "AVG Do Not Track" with the same ID as the 1.0.0.2 version available from AMO. The AMO one was released more recently so the version numbers are unfortunate.

We need to blocklist this 14.0.3.x version before more people upgrade from the earlier versions to the one that breaks us. Once people are broken I'm not sure how you trigger component re-registration these days to rebuild the broken category manager.
Group: addons-security → client-services-security
Component: Add-on Security → Blocklisting
Keywords: sec-high
(In reply to Michael Coates [:mcoates] from comment #5)
> Can you share their response? Are they planning to take action? Also, when
> you say "our side" are you referring to adblockplus or Mozilla?

I'm referring to Adblock Plus, we were able to reach out to Guy Zipori (Director of Engineering at AVG). For some reason they won't use Bugzilla so I had to send them this bug report as an HTML page. The response I got is simply: "Thanks Wladimir, I'm forwarding this to my team." Doesn't say much unfortunately and I don't have anybody to contact on a lower level (who would presumably be able to tell something about the timeline). Will try to follow-up again today.

(In reply to Daniel Veditz [:dveditz] from comment #6)
> Another oddity: the statistics pages show only 11,000 downloads total, but
> show a user-count in the millions.

The user counts are probably exaggerated. The way I understood their update mechanism, each time update is triggered (on each browser start? not sure) they first try an AMO update and only fall back to their custom update if it fails to find anything. So their users produce a lot more update checks on AMO than normal. Still - AVG installs all their extensions along with they applications, whether hosted on AMO or not.

(In reply to Daniel Veditz [:dveditz] from comment #7)
> We need to blocklist this 14.0.3.x version before more people upgrade from
> the earlier versions to the one that breaks us.

This will of course disable their update mechanism - so people having their 14.0.3.x version will not get updated unless that update comes with an application or from AMO (requires uploading the extension there in the first place and passing review). Not sure whether that's a bad thing...
(In reply to Daniel Veditz [:dveditz] from comment #7)
> I'm
> not sure how you trigger component re-registration these days to rebuild the
> broken category manager.

I just verified that the breakage isn't persistent. Since AVG toolbar isn't restartless, uninstalling it triggers component re-registration and the category gets populated again.

In Adblock Plus we now listen for category notifications and add our category entries back if they are removed. Speaking of desperate measures...
Kris, can you please share the current usage numbers for the AVG Toolbar?
It has roughly 11M users.
(In reply to Kris Maglione [:kmag] from comment #11)
> It has roughly 11M users.

This probably needs to be divided by a factor 2-3, see my notes on their update mechanism in comment 8. Still lots of users.
Those numbers come from startup ping data, which as far as I know they don't inflate.
(In reply to Wladimir Palant from comment #8)
> (In reply to Daniel Veditz [:dveditz] from comment #6)
> > Another oddity: the statistics pages show [...]
> 
> The user counts are probably exaggerated.

We're talking about two different things I think. AMO hosts a Do Not Track add-on with a standard GUID-style ID, which also appears to be a part of some suite/package based on the number of people who have it (and the differing, odd, versions).

You linked to an add-on with a different ID ("avg@toolbar"). It seems to have incorporated code from the Do Not Track add-on based on similar file names, maybe based on an older version before AMO review forced some changes in the code.

My comment 7 was poorly worded. The first paragraph was a continuation of comment 6 about the "AVG Do Not Track" add-on. The second paragraph asking for a blocklist was for the "AVG Security Toolbar" add-on--containing broken DNT functionality--that this bug is about.
 
> (In reply to Daniel Veditz [:dveditz] from comment #7)
> > We need to blocklist this 14.0.3.x version before more people upgrade
> > from the earlier versions to the one that breaks us.
> 
> This will of course disable their update mechanism

Well... yeah. I assume people didn't go install the add-on by itself but that it came with something like their security suite. You did say it was the premium people who seemed to get this new version? If so maybe that products update mechanism can install a newer version for them.

Frankly I don't think breaking their (insecure looking) update mechanism is our problem -- they are breaking Firefox and disabling security mechanisms. Especially inexcusable in a "security product".
(In reply to Daniel Veditz [:dveditz] from comment #14)
> Frankly I don't think breaking their (insecure looking) update mechanism is
> our problem -- they are breaking Firefox and disabling security mechanisms.
> Especially inexcusable in a "security product".

I agree. I think we could make a case for blocking the add-on based on the update mechanism alone. From a security company this is... inconceivable.
(In reply to Kris Maglione [:kmag] from comment #11)
> It has roughly 11M users.

Do we know the numbers by version? What percentage of those are on the 14.0.3.* version with DNT disabled by default, vs older versions 14,13, and 12?

The 13.2.0.5 version has the same broken code, just waiting for users to disable the DNT functionality to strike.

jar:http://toolbar.avg.com/partners/avg/13.2.0.5/download/avg-ie.xpi!/components/avg-dnt-policy.js

(you need to toggle the pref network.jar.open-unsafe-types to open that URL, otherwise download the .xpi locally if you want to see the contents)
(In reply to Daniel Veditz [:dveditz] from comment #16)
> Do we know the numbers by version? What percentage of those are on the
> 14.0.3.* version with DNT disabled by default, vs older versions 14,13, and
> 12?

I don't have it at hand, but since the add-on doesn't specify an
updateURL I may be able to get it.
14.0.3.* is still relatively low, with roughly .5M users. We should probably block it ASAP before the 7.6M users on 14.0.2.* are upgraded.
Depends on: 840445
Filed bug 840445 as an explicit blocklisting request.
(In reply to Wladimir Palant from comment #8)
> The response I got is
> simply: "Thanks Wladimir, I'm forwarding this to my team." Doesn't say much
> unfortunately and I don't have anybody to contact on a lower level (who
> would presumably be able to tell something about the timeline). Will try to
> follow-up again today.

Unfortunately, I don't have any news - I didn't receive any further response. So blocklisting it is...
Adding Guy Zipori from AVG. It looks like they're planning on releasing a fix soon, so blocklisting might not be necessary. I'll let him update us with the timeline.
Dear All,

We have been notified about the bug and took immediate action to resolve it in a high priority.

A new version with a fix was already developed and submitted to QA yesterday. ETA for launch is Friday (Feb 15).

I will continue to update on the progress.

Thanks,
Guy Zipori
Director of Engineering
AVG Technologies
According to AVG, the fixed version was released as planned and users are currently being updated. Can someone please confirm that the new version addresses all the issues brought up on this bug?
(In reply to Jorge Villalobos [:jorgev] from comment #23)
> According to AVG, the fixed version was released as planned and users are
> currently being updated. Can someone please confirm that the new version
> addresses all the issues brought up on this bug?

The update backend mentioned in comment 3 now serves up http://toolbar.avg.com/partners/avg/14.2.0.1/download/avg-ie.xpi (version 14.2.0.1). Function disableDNT has been corrected, it now uses categoryManager.deleteCategoryEntry(). Other issues mentioned here (serving updates via plain HTTP and using @mozilla.org/content-policy;1 as contract ID for the own component) have not been addressed.
Guy, we need you to look into these issues:

1) Updating over plain HTTP. If toolbar updates are being pushed this way with no additional verification, they are a very tempting target for MITM attacks and malicious add-on injection.

2) Registering your component as "@mozilla.org/content-policy;1" is confusing and could potentially conflict with Firefox code in the future. You should use something like "@avg.com/content-policy;1".
Flags: needinfo?(guy.zipori)
Regarding item #1
The request for updates is using a plain HTTP however we do have mechanisms to protect from MITM attacks. For example, the updater module that is responsible for updating the add-on is checking signatures inside the package before performing the update.

Regarding item #2
We will change the code for the upcoming version as your suggestion.
The next version is planned to be released in early March
Flags: needinfo?(guy.zipori)
Alright, calling this fixed then.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → Toolkit
Shouldn't this and its blockers be made public by now?
This one and one of the dependencies have usage data that I'd rather not share, even if it's old. I opened bug 840445.
Group: client-services-security
Derp.
Group: toolkit-core-security
(In reply to Jorge Villalobos [:jorgev] from comment #29)
> This one and one of the dependencies have usage data that I'd rather not
> share, even if it's old. I opened bug 840445.

Now that blocklisting comes under the toolkit product this bug is going to continually get flagged by the "old security bugs to unhide" queries. Are there specific comments we can hide that would let us open the rest of the bug before it's accidentally unhidden in some larger sweep? comment 18 obviously but are there others?
Group: toolkit-core-security → core-security-release
Flags: needinfo?(jorge)
On this bug, comment #11, comment #16, comment #18. Related: bug 840274 comment #1 and its attachment.
Flags: needinfo?(jorge)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.