Closed Bug 326579 Opened 14 years ago Closed 14 years ago

At least one 'effectivebrand' toolbar on AMO - fetches remote dynamic js

Categories

(addons.mozilla.org Graveyard :: Public Pages, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: Gijs, Assigned: shaver)

References

()

Details

Attachments

(1 file, 1 obsolete file)

The "A torrent search toolbar" extension (https://addons.mozilla.org/extensions/moreinfo.php?id=1496) is done using the toolbar generator found on the URL of this bug. This means it loads a pretty big javascript from a weblink. The extension seems to have a builtin capability to update this whenever it deems necessary.

What's more, after googling along from 'EffectiveBrand' their website (http://www.google.com/search?hl=en&q=%22an+EffectiveBrand+toolbar%22), there are a couple of links with disturbing screenshots of the toolbar in question. They show items like "Cool links" (http://www.google.com/search?hl=en&q=%22an+EffectiveBrand+toolbar%22), which allegedly contain links relative to the pages you're currently visiting (see also http://www.serenityfound.org/toolbar.html - check the source of both of these pages for the 'an EffectiveBrand toolbar' notice). To me, that implies they're sending the current url off to some database to check for related links (assuming such a database would be too big to keep on the user's machine). That would be considered spyware in my book.

Regardless of whether it really is spyware, fetching javascript content over the web is dangerous and should not be allowed (and isn't, to my knowledge). I'm guessing there are probably other extensions currently listed on AMO that use the same toolbar generator (I've seen so many community-related extensions in the queue of late...).
For reference:

[9-2-2006 22:16] <Pontus_> Hannibal: Seems like trovando toolbar is doing the same as well. I recall discussing effective brands toolbar with someone here earlier. Thought that person would follow up though, maybe we misunderstood each other. Anyway, IIRC, these toolbars are registered on different authors but all the authors have e-mail addresses @effectivebrands, so it should be rather easy to find suspectible extensions.

(from #umo on moznet)
Also effectivebrand.com toolbars:
https://addons.mozilla.org/extensions/moreinfo.php?id=1734
https://addons.mozilla.org/extensions/moreinfo.php?id=1889

Notice also that various people have tried mentioning this, and all the comments were deleted. Maybe we should be more critical to the extension authors when removing comments? They might be justified in various cases :-)
This seems pretty straightforward: it's violating our policy, we should pull the extension(s).

(We have posted that somewhere at last, haven't we?)
(In reply to comment #0)
> To me, that implies they're sending the current url off to some database
> to check for related links

I don't have a problem with that per se as long as the extension clearly discloses that part of its functionality, and that it's truly part of the value of the extension and not just tacked-on tracking.  The Netcraft anti-phishing toolbar does something along those lines, as does the Googlebar in order to display pagerank.
https://addons.mozilla.org/extensions/moreinfo.php?id=1346

More of the same stuff. As before, lots of users complain about this toolbar as well...
Where are we with this?  Can someone point me to the latest policy document that governs these things, and update me on what we've done with this extension (family) so far?
http://wiki.mozilla.org/Update:Requirements/LegalAndReview
Is the best I could find. It does not mention remote JS (though one could argue it is subject to the "Do no evil" approach). For what it's worth, none of those rules are really enforced either. There are plenty of (popular) extensions on AMO that have closed-source components, for example.

What we've done so far: 'nothing'.
What I've been trying to do: get a complete list of these extensions for starters.

Morgamic: I'm wondering if there are any other documents. They're not clearly linked on the wiki, if that is the case.
There is no "official" policy document, and the only real "draft" I've seen was what we found in the wiki in comment #7.  I believe it's been there for quite a long time -- probably 8 months...

Our better judgement tells us we should disable these toolbars because of complaints, feedback and their open-ended nature.  Let's do it.  Any objections?

The extension family comes close to crossing the "It must not do anything Evil." clause, but maybe we should add something about how JS includes should be utilized, if at all to set a better precedent for future cases.
Status: NEW → ASSIGNED
Grep is finding:
Movie (https://addons.mozilla.org/extensions/moreinfo.php?id=1625)
Diggbar (https://addons.mozilla.org/extensions/moreinfo.php?id=1352)
DxCurrencies (https://addons.mozilla.org/extensions/moreinfo.php?id=1604)
Email Notifier (https://addons.mozilla.org/extensions/moreinfo.php?id=2052)
FFAddonsBar (https://addons.mozilla.org/extensions/moreinfo.php?id=1660)
HOT-IL (https://addons.mozilla.org/extensions/moreinfo.php?id=1889)
Israel Radio (https://addons.mozilla.org/extensions/moreinfo.php?id=1995)
Israel Radio (Hebrew Version) (https://addons.mozilla.org/extensions/moreinfo.php?id=1997)
Reel New Media (https://addons.mozilla.org/extensions/moreinfo.php?id=1519)
SendMeFile (https://addons.mozilla.org/extensions/moreinfo.php?id=1078)
TorrentSearch (https://addons.mozilla.org/extensions/moreinfo.php?id=1496) 
Guitar Tabs (https://addons.mozilla.org/extensions/moreinfo.php?id=1626)
The Marker (https://addons.mozilla.org/extensions/moreinfo.php?id=1734)
Torrentools (https://addons.mozilla.org/extensions/moreinfo.php?id=1346)
Trovando (https://addons.mozilla.org/extensions/moreinfo.php?id=1140)
Webpedia (https://addons.mozilla.org/extensions/moreinfo.php?id=1232)
World cup 2006 (https://addons.mozilla.org/extensions/moreinfo.php?id=1910)
(I removed the redundant word 'Toolbar' to fit these on one line, or such was the plan, anyway).

These things seem to always use jar files. I've searched all xpi's with jar files in them, I'll try searching the remaining ones when I have more time, but I expect this should be everything.

Method used, if anyone wants to do it again:
Grab all xpi's from ftp.mozilla.org. (I did this 1-2 weeks ago, so if I'm unlucky new ones have been added since then)
Use a bash script to extract all the xpi's into their own folder.
Use another bash script to extract all the jar files into their own folder.
Grep for 'effectivebrand', and save the result to a textfile.
The details of the policy document are less important than determining what we want to do with this class of extension, and doing it; our policy can't constrain us from doing what we think is right, or we have a severe policy bug.  Getting a list of those extensions would be a great start, thanks for volunteering to do that.

(I agree that we should figure out how to make the review process more consistent and complete, but that's probably best dealt with distinctly from this issue, and learning from our decisions here.  I'm not entirely convinced that "loads remote JS" is a good criterion for evaluation, especially given the use of JSON web services, but it might be.)
Attached file Draft Email (obsolete) —
I've written a draft email to the community about these extensions.  I'd like to remove these sometime on Friday night, barring any objections.

Sound good?  Let me know what you think about the email, or if there is any reason why we should pause and rethink deleting these extensions.

For discussion:
* Should we talk to effectivebrand.com reps and work something out?  Why or why not?
* We will get some backlash from the extension devs, but oh well, right?
* Anything else I'm missing here?
Attachment #214563 - Flags: first-review?(shaver)
Yes, we should talk to effectivebrand.com and explain why we feel we can't host toolbars made with their application (because of remote script loading?).

From my reading of the effectivebrand documents and (very) light inspection of the applications, there is nothing other than the remote script updating that is inherently "evil" about applications built with their system.  If the extension sends URL or other user data to a remote server, it should say so clearly to the user, and then that's fine.  We don't consider the Google SafeBrowsing extension to be evil, for example, because it clearly describes those cases.

And their own policy, as I linked in an earlier comment, indicates that they have similar requirements to ours, so I think it would be worthwhile to contact them and discuss the matter.

If we're going to remove extensions because of negative comments about user privacy and such, we need to not paint all of the extensions made with the same tool with that brush.  If we're removing them because of the remote-script-loading issue, then we should stick to that reason and not muddy it with references to unspecified (and unverified?) user feedback.

Also: why is this is marked security-sensitive?  It seems like there's nothing in this bug that needs to be kept secret to protect anyone's security.  If anything, the information in this bug can only _help_ users avoid installing these extensions, not assist someone in exploiting anything, etc.
Attachment #214563 - Flags: first-review?(shaver) → first-review-
Okay, thanks Mike.  I agree with your comments, and would prefer we did talk to them -- in spirit of community, etc.

My question would then be:
* who will talk to them
* how?  email, phone, teleconference, etc.
* what topics should be discussed
* what would be our goal(s) to be achieved by working with them

Attachment #214563 - Attachment is obsolete: true
This is a policy discussion, not a security-sensitive issue.  The more participants, the better.
Group: update-security
CC list accessible: false
Not accessible to reporter
One point I've seen come up is that extensions that do things like XmlHttpRequest and Eval should be scrutinized more closely.  Perhaps it would help (this should probably be another bug) if some of these things could be automatically determined by the addons system when the extension is uploaded, and list them for the reviewer so they know what to take a closer look at.
I'm fairly sure that at least a fair amount, if not most, of the reviewers are not capable of actually determining whether or not such code would be considered 'evil' or not. I'm not sure I myself would be, even, if the extension used custom xbl stuff, for example (about which I know just about nothing). For those who are, it would slow down the sluggish review process even more. It'd also make it less of a community thing, because the people who would review these extensions would be highly technically skilled (compared to Joe User) and would need a good understanding of the mozilla platform. After all, in chrome, one could also use XPCOM to do (more) evil things that you could do with XMLHttpRequest. That means the people who would be able to review extensions would be a (small) subset of the current set of reviewers.

A quick check tells me AMO currently has 130 extensions in its queue (and a good 30 comments flagged). That's simply not reviewable with such a small subset of developers. And how would you reject such applications? To me, it seems that just checking specific language features because they might be used for evil things is not a course of action that's adequate for the problem at hand.
Distinct issues here:

- what are the characteristics of extensions that would disqualify them from our hosting?

- how well can we detect those characteristics before they're hosted?

- how much cost (review lag, f.e.) are we willing to bear?

- what other options do we have for reducing that cost?  (trust network?  fast-path for established extension authors?  tools to detect cases that require additional attention, to reduce the number of cases that have to go through the smaller group of more technically-skilled reviewers?  training for reviewer candidates?)
I've posted a bit about what EffectiveBrand claims and what the toolbars do on my site if anyone's interested. <http://rdmsoft.com/r/blog/340>

As for what AMO can do, I'm not sure. There really needs to be clear policy on binary components and remote loading. An extension may want to load/update news, currency data, time zones, etc., but at what point does it become Bad? I've seen one or two extensions that are basically glorified bookmarklets, and those bookmarklets add remote script elements to the current page. In fact, one of the buttons on the Kaboodle toolbar (current featured extension) seems to do this. It's arguably safer than eval()ing in chrome, but is it still something we want to allow?

After a quick look, I found one extension that eval()s the result of an XMLHttpRequest - BetterShopper <https://addons.mozilla.org/extensions/moreinfo.php?id=740>.
Some level of manual code review would help in this situation, though I doubt it's practical for all extensions. Automatically detecting feature (mis)use is possible, but it's unlikely to stand up to a developer who knows about it.
I just got an email off the AMO Reviewer/Admin list that the Torrenttools toolbar has been individually removed for this reason, by request of a user (or reviewer) who noticed this, apparently. CC-ing Alan.
eval of XMLHttpRequest is quite common in web apps these days, for accessing JSON APIs, so we certainly don't want a blanket restriction there; on that point I disagree quite forcefully with comment 0's last paragraph.  What we care about is behaviour and the "informedness" of the user's consent to that behaviour.

A "watchlist" of patterns or API calls would help us fastpath more reviews with higher confidence, and help us focus our detailed review attention on ones that need that.

Removing individual extensions because they have bad user behaviour (including simply under-informing users about their behaviour) is the right behaviour, generally, though I'm not familiar enough with the process to say whether I think we do it well today.

morgamic: I can be the point of contact for communcation with effectivebrands.com, but I won't be able to do much with this problem until Tuesday of next week or so.
Has there been any progress on this?
Same question comment #22 asked really. Where are we now? Do we have a policy yet? Is there a meeting scheduled where we discuss a policy? Has anyone talked to the effectivebrand people yet?

Michael, if you're busy, please look for someone else on whose plate you can chuck this bug, preferably someone employed by MoFo/MoCo, as they have the authority to contact effectivebrands on these matters, and actually make a difference.
(In reply to comment #21)
> eval of XMLHttpRequest is quite common in web apps these days, for accessing
> JSON APIs, so we certainly don't want a blanket restriction there; on that
> point I disagree quite forcefully with comment 0's last paragraph.  What we

I thought comment 0 talked about eval()ing JS code from within privileged code. This is what the "torrent search toolbar" does, and I believe it is very evil. It is as bad as (if not worse than) an external updateURL in install.rdf.

Running eval() on results of a remote XMLHttpRequest can not be a criteria of course, but it should be one of the things that prevent an extension from being accepted on AMO.
The latest version of the EffectiveBrand code does not appear to load JS remotely, however it does place its own updateURL in install.rdf. It also continues to spy on search queries and other page loads without permission.

The extensions listed in comment 9 are now a mixture of the old code and the new stuff. I believe all of them should be removed from AMO.
AMO has an edit that prevents off-site updateURL and has since the checkin on 2006-01-10 18:59 (from bug 281902).  
http://lxr.mozilla.org/update1.0/source/developers/additem.php#109
Ok, I've re-checked, and the install.rdf I checked before was from an XPI downloaded directly from EffectiveBrand's site. The ones on AMO have had it removed, so that part's working. :)
morgamic is going to nuke the extensions, I'm going to mail the owners tomorrow.  We should go "default deny" on these sorts of behaviours, I'm now convinced, and let extension authors plead their case if they need an exemption (such as use of a JSON service).
Assignee: morgamic → shaver
Status: ASSIGNED → NEW
Ok, guys, given these two things brought up by tH:
http://worms.rdmsoft.com/temp/ebreview.png
http://rdmsoft.com/r/blog/340

It's become obvious that our suspicions were correct and we should proceed with deleting these toolbars and prevent their submission in the future.

Here is our TODO list:
* get an updated list of effectivebrand addons (Hannibal, could you help with this?)
* remove all effectivebrand toolbar extensions from AMO (morgamic)
* remove all effectivebrand files from stage (morgamic, infra)
* revoke reviewer status for any reviewers who were focusing on approving only AMO accounts (morgamic)
* email reviewers to notify them that effectivebrand toolbars should not be approved (morgamic)
* email owners of efffectivebrand toolbars explaining why this happened (shaver)

(In reply to comment #29)
> * revoke reviewer status for any reviewers who were focusing on approving only
> AMO accounts (morgamic)
That's "focusing on approving only effectivebrand toolbars".  Typo.
tH: can you send me, possibly out-of-band, more information on the "search query spying"?  (Thanks for all your work on this!)
I've uploaded some of the data sent and received to <http://worms.rdmsoft.com/temp/ebtrace.txt>, a summary:
- When the browser starts, a "login" request is sent with the toolbar ID and a unique user ID. The server echoes this information back, and sets a session cookie.
- Whilst the latest version of the toolbar code (1.0.1.10) does not load remote JS, it does load a huge chunk of XML at startup that defines the toolbar layout. I haven't checked the code, so I don't know exactly what instructions can be given here.
- Once it's loaded, it basically tells the server whenever I do anything with the toolbar, from searching to opening the About dialog.
This should be everything. There were two toolbars that were on the FTP but which I couldn't find back on AMO. Any clues as to where those are? I have a hunch they've been removed/denied, just not off the FTP server - can anyone confirm?
(In reply to comment #33)
> I have a hunch they've been removed/denied, just not off the FTP server - can
> anyone confirm?
It was probably a user-deletion.  They don't exist on the admin side, that's for sure.

I've gone through the list and removed all of the database entries, but because of the bug I found in addon deletion, we'll have to remove them from FTP manually until bug 334992 is fixed.  :\
I'm marking this FIXED, as the toolbars at issue have been removed, and I had a conversation today with EffectiveBrand folks that makes me quite optimistic that we'll have a good story for their toolbars in the near future.
Let's all pretend I know how to use bugzilla, K?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
I know this issue is marked fixed but I think the older versions of the toolbars need blacklisted so that the toolbars are disabled on user's that haven't updated yet. 

Sorry for the spam if they have been blacklisted and I just somehow missed that.
Blacklisting isn't implemented in any released version of Firefox, and even if it were we don't want to keep people from having the extension installed, it just wasn't suitable (prior to 1.0.1.13) for hosting on AMO.
AMO BUGSPAM FOR COMPONENT MOVE AND DELETE (FILTER ME)
Component: Listings → Web Site
AMO BUGSPAM FOR QACONTACT FIX (FILTER ME)
QA Contact: listings → web-ui
As a side-note, EffectiveBrand are the company behind UCmore, which was a toolbar which at one point was widely bundled without user permission, and had enough privacy problems to qualify as spyware.

After noting the issues I raised with their software they worked to fix it and got an endorsement from me for the non-spyware version of their software - and then six months later went straight back to installing their stuff from IE security hole exploits.

Not a trustworthy company.
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.