735931 - avast! WebRep add-on is obnoxious, possibly insecure?

Status: RESOLVED INVALID

(Toolkit :: Blocklist Policy Requests, defect)

Description

[Nicholas Nethercote [inactive]]

avast! WebRep ranks highly in the list of installed add-ons.  In the top 100 add-on testing (bug 730737) Nils Maier observed the following things about it (in https://etherpad.mozilla.org/hSOVNzKZen).

- about:compartments showed some compartments after closing the
  page which I initially thought where Zombies. However these
  compartments all disappeared after 1-2 minutes.

- The add-on uses DOM Modification events, which are notorious for
  degrading browser performance.

- The add-on injects some local scripts and css into EACH page,
  which might conflict with the website.

- The add-on declares a huge number of globals in browser.xul, which
  might conflict with other add-ons.

- The add-on modified Date.prototype of browser.xul, which might
  conflict with other add-ons.

The third point sounds similar to bug 721264 comment 5 -- though I'm far from certain that it is the same -- so I'm marking this security-sensitive.

Even if it turns out not to be a security risk, the behaviour above sounds quite obnoxious, and much more invasive than other anti-virus add-ons.  Even though I filed this under the "blocklisting" component I suspect "obnoxious" isn't enough to meet the blocklisting criteria.  But I wonder if we can reach out to Avast and ask them to tone it down, or something.

FWIW, I tried to install the add-on to reproduce Nils' testing and failed, because the installation totally borked my Windows 7 partition and it took numerous safe mode reboots disk repair utility runs to uninstall it and get things back to normal.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> - The add-on uses DOM Modification events, which are notorious for
>   degrading browser performance.
DOM Mutation events perhaps, not Modification

Comment 2

[Nils Maier [:nmaier]]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> The third point sounds similar to bug 721264 comment 5 -- though I'm far
> from certain that it is the same -- so I'm marking this security-sensitive.

The scripts are local (shipped with the extension) and hence do not pose security risks regarding MITM and such.
Looks like this:
  var hoverdivscript = doc.createElement("script");
  hoverdivscript.id = "wrc-script";
  hoverdivscript.setAttribute("type", "text/javascript");
  hoverdivscript.setAttribute("language", "JavaScript");            
  hoverdivscript.innerHTML = MOUSEOVER_SCRIPT.replace( /\$NORATING/g , this.strings.getString("no-rating")) // more l10n replacements
  head.appendChild(hoverdivscript);
where MOUSEOVER_SCRIPT is just a local js string containing the code.

Since these scripts will be accessible from the website, however, there is potential for the website hijacking or disabling the functionality these scripts will provide.
I see functions like isCreditCardNumber(), which seems to be used to raise alerts regarding phishing, which a phishing site could potentially disable.

I recommend that some AMO super-reviewer does a proper review of the add-on and gets in touch with avast.

(In reply to Olli Pettay [:smaug] from comment #1)
> (In reply to Nicholas Nethercote [:njn] from comment #0)
> > - The add-on uses DOM Modification events, which are notorious for
> >   degrading browser performance.
> DOM Mutation events perhaps, not Modification

Indeed, Mutation, not Modification, of course. In particular, the add-on uses DOMNodeInserted.

Comment 3

[Andrew Williamson [:eviljeff]]

(In reply to Nicholas Nethercote [:njn] from comment #0)
> - about:compartments showed some compartments after closing the
>   page which I initially thought where Zombies. However these
>   compartments all disappeared after 1-2 minutes.

Seems okay to me

> - The add-on uses DOM Modification events, which are notorious for
>   degrading browser performance.

It does, but these are often unavoidable for tracking certain types of dynamic page modification by scripts used by the website.  We (reluctantly) allow this in fully reviewed addons as long as they're not registered on the browser chrome.

> - The add-on injects some local scripts and css into EACH page, 
>   which might conflict with the website.

Not that uncommon.  Not really advisable either, though more because a malicious website could override the functionality.  We would allow for preliminary review.
 
> - The add-on declares a huge number of globals in browser.xul, which
>   might conflict with other add-ons.

We would allow for preliminary review.

> - The add-on modified Date.prototype of browser.xul, which might
>   conflict with other add-ons.

We would allow for preliminary review.  

> The third point sounds similar to bug 721264 comment 5 -- though I'm far
> from certain that it is the same -- so I'm marking this security-sensitive.

Its not.  The addon in bug 721264 was inserting scripts retrieved over http into a chrome privileged scope.  This addon is inserting locally stored scripts in content scope.

> Even if it turns out not to be a security risk, the behaviour above sounds
> quite obnoxious, and much more invasive than other anti-virus add-ons.  Even
> though I filed this under the "blocklisting" component I suspect "obnoxious"
> isn't enough to meet the blocklisting criteria.  But I wonder if we can
> reach out to Avast and ask them to tone it down, or something.

From a brief look, IMO, its not terribly written and would be approved for preliminary status if submitted to AMO.  So I don't believe it would fit our blocklisting criteria.

> FWIW, I tried to install the add-on to reproduce Nils' testing and failed,
> because the installation totally borked my Windows 7 partition and it took
> numerous safe mode reboots disk repair utility runs to uninstall it and get
> things back to normal.

I can send you the xpi if you want - I have Avast installed.  (I've not got the addon enabled but I xpi'd it up out of the program files folder to test)

Comment 4

[Nils Maier [:nmaier]]

Attachment: Tested XPI

For reference, this is the avast! add-on that is shipped with the AV-software (trial version).
It is pure JS and does not contain binaries.
You could review the code, however, for it to fully work you'd need to install the actual software, as the add-on queries some stuff from a localhost server the main software provides.

Comment 5

[Nils Maier [:nmaier]]

Re: Andrew

I concur, that unless somebody finds additional, not yet discovered security issues, the add-on right now would be "preliminary review".

Comment 6

[Brian King [:kinger]]

What a coincidence! Today as this bug becomes active again, I find out it is the cause of a Firefox crash with an add-on I am working on (https://addons.mozilla.org/addon/video-downloader-player).

Well, it could also be bad code in my add-on, but alas.

Comment 7

[Jorge Villalobos [:jorgev] (he/him)]

So, this is neither a security problem nor a worthy of blocklisting.

Andrew, please review the add-on and see if there's anything we should be concerned about.

Brian, do you have any steps to reproduce this crash?

Comment 8

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Jorge Villalobos [:jorgev] from comment #7)
> nor a worthy of blocklisting.

Grammar bustage :(

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Have we tested how bad performance problems the addon cause?
At some point we did blocklist Skype because it was using mutation event listeners and caused
horrible performance problems.

Comment 10

[Andrew Williamson [:eviljeff]]

(In reply to Jorge Villalobos [:jorgev] from comment #7)
> Andrew, please review the add-on and see if there's anything we should be
> concerned about.

I did earlier.  There are namespace & prototype issues that may affect other addons as mentioned in the comment 0 and the script insertion could affect some webpages if they happen to use the same function names or DOM IDs.  But nothing overly concerning.

Comment 11

[Brian King [:kinger]]

(In reply to Jorge Villalobos [:jorgev] from comment #7)
> Brian, do you have any steps to reproduce this crash?

Yes and it is 100% reproducible.

1. Install Avast antivirus
2. Launch the 'Player' feature of the downloader add-on

The crash does not invoke the Crash Reporter thus no useful stack is available.

But as far as I can see so far, it is not the add-on that is the culprit, but the antivirus system itself. If I turn it off (disable the shields), the crash goes away. My hunch right now is that it is intercepting some disk access that the downloader add-on is doing.

Comment 12

[Jorge Villalobos [:jorgev] (he/him)]

Yeah, it might be trying to intercept the download. As far as I can tell, Ant uses the Download Manager correctly. Have you tried starting regular downloads? Do those work ok?

Comment 13

[Brian King [:kinger]]

(In reply to Jorge Villalobos [:jorgev] from comment #12)
> Yeah, it might be trying to intercept the download. As far as I can tell,
> Ant uses the Download Manager correctly. Have you tried starting regular
> downloads? Do those work ok?

No, it it not downloading triggering it, just loading the Player with the list of already downloaded videos. Regular Firefox download works fine.

Comment 14

[Robert Kaiser]

Brian, uh, this so awfully sounds like it's connected to a recent rise in crashes that we could actually get reports for, see bug 733892. Someone from QA and/or with a debugger should get behind this and find out what's going on there.

Comment 15

[Nicholas Nethercote [inactive]]

Thanks for the detailed analysis, Andrew.  I'll defer to your expertise!

Comment 16

[Scoobidiver (away)]

(In reply to Robert Kaiser (:[e-mail]) from comment #14)
> Brian, uh, this so awfully sounds like it's connected to a recent rise in
> crashes that we could actually get reports for, see bug 733892.
All crash signatures correlated to Avast have a low correlation, 25% at worst, and indicate that it affects European users, such as Russian users are correlated to Yandex Toolbar and Windows XP users to Java Quick Starter.

Comment 17

[Robert Kaiser]

Scoobidiver, I never said that avast! would be the culprit for all those. But this is a crash with downloads and potentially virus scanning involved, and that's what the other one seems to point to as well. It's probably NOT specific to one single security suite, but this instance that is reproducible could, when being looked at with a debugger, potentially give a clue as to who we have seen a rise of such problems.