522522 - Lightweight themes should not use the xpinstall whitelist for web content trusting

Status: RESOLVED WONTFIX

(Firefox :: General, defect)

Description

[Reed Loden [:reed]]

Bug 511771 added support for web content being able to install lightweight themes. This included modifying the xpinstall whitelist to add getpersonas.com. I consider this an abuse of the xpinstall whitelist, as lightweight themes are not the same thing as add-ons and should not be treated with the same level of trust. A separate whitelist just for lightweight themes should be created and the xpinstall whitelist should have getpersonas.com removed from it.

Comment 1

[Dave Townsend [:mossop]]

I don't think it really makes much sense for these to be separate. We are trying to make Lightweight Themes look like just another type of add-on to users so I think it would be confusing to have different permission lists for them.

Comment 2

[Mike Connor [:mconnor]]

I agree strongly with comment 1.  Further, I also disagree that it's a matter of trust.  The whitelist, at this point, is basically "you can do annoying things to the user, like pop modal install dialogs, or change the persona of the browser"  I could make a case for either being more intrusive/annoying, tbh, and since the whitelist doesn't let you bypass the install dialog, I don't actually see any harm.  This is, of course, what we discussed before we did it.

Comment 3

[Reed Loden [:reed]]

I'll bring this back up at the security review for lightweight themes, as I strongly disagree.

Comment 4

[Mike Connor [:mconnor]]

Or, well, you could make a concrete argument for how this conflation is harmful, or relevant to security at all, instead of just bandying about terms like "this is an abuse of the xpinstall whitelist"

Put another way: explain, in detail, how this will cause harm or expose users to greater harm than is permissible.  You're just asserting "this is bad" without providing concrete examples of why it is bad.

Comment 5

[Justin Scott [:fligtar]]

I strongly agree with Reed that these should be separate. 2 examples so far:

1) getpersonas.com was added to the add-ons whitelist and they were not informed of the responsibilities of being on this whitelist. Thus, bug 521076

2) mozilla.com firstrun and whatsnew pages need to be able to preview Personas, which either means adding mozilla.com to the whitelist or using iframes to a site that is on the whitelist. We can't add mozilla.com to the whitelist because it adds *.mozilla.com to the whitelist, meaning people linking in blog comments to an add-on would be pretty successful.

We could add mozilla.com to the whitelist and make our iframe problems go away if there was a Personas-only whitelist. I discuss these and other issues that have now become a huge deal in bug 522513.

Comment 6

[Reed Loden [:reed]]

Bug 521076 is basically what I was worried about happening because of this. :/

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #5)
> I strongly agree with Reed that these should be separate. 2 examples so far:
> 
> 1) getpersonas.com was added to the add-ons whitelist and they were not
> informed of the responsibilities of being on this whitelist. Thus, bug 521076

That is more of a communication problem than actually a reason not to use the
same list.

> 2) mozilla.com firstrun and whatsnew pages need to be able to preview Personas,
> which either means adding mozilla.com to the whitelist or using iframes to a
> site that is on the whitelist. We can't add mozilla.com to the whitelist
> because it adds *.mozilla.com to the whitelist, meaning people linking in blog
> comments to an add-on would be pretty successful.
> 
> We could add mozilla.com to the whitelist and make our iframe problems go away
> if there was a Personas-only whitelist. I discuss these and other issues that
> have now become a huge deal in bug 522513.

If we added mozilla.com to a lightweight themes whitelist then blog posts would
be able to change a user's browser skin trivially. I'm not sure this is a good
thing at all.

Comment 8

[Reed Loden [:reed]]

(In reply to comment #7)
> (In reply to comment #5)
> > 1) getpersonas.com was added to the add-ons whitelist and they were not
> > informed of the responsibilities of being on this whitelist. Thus, bug 521076
> 
> That is more of a communication problem than actually a reason not to use the
> same list.

Not really... Allowing anybody to misuse getpersonas.com to bypass the warning bar is a real security problem, as it basically makes the warning bar completely useless.

> If we added mozilla.com to a lightweight themes whitelist then blog posts would
> be able to change a user's browser skin trivially. I'm not sure this is a good
> thing at all.

I think fligtar really just means www.mozilla.com and the various locale-based subdomains under it, not all of *.mozilla.com.

Comment 9

[Justin Scott [:fligtar]]

(In reply to comment #7)
> If we added mozilla.com to a lightweight themes whitelist then blog posts would
> be able to change a user's browser skin trivially. I'm not sure this is a good
> thing at all.

I am far less concerned about what people with permission to use JS on mozilla.com sites do than about what people who can put a link on mozilla.com sites do.

Which is another reason the two add-on types aren't equals: one just requires a simple link, while the other requires JavaScript events to be triggered.

Comment 10

[Mike Connor [:mconnor]]

Ultimately, this comes down to a couple of issues:

1) is it possible to harden getpersonas.com sufficiently against ways to bypass the whitelist?

2) what is the purpose of the whitelist, and does expansion of the whitelist expose users to harm?

Answers:

1) Sure, we did it for AMO.  That said, 2) is the reason I'm not concerned here.

2) The whitelist _only_ exists to prevent sites from spamming users with unrequested/repeated install dialogs in a modal dialog loop, forcing them to accept the install to escape the loop.  It does not allow automatic installation, or permit sites any additional privileges.  Unless getpersonas.com allows arbitrary script injection, the only "weakness" here is that direct links to XPIs from that site will not trigger the warning bar when a user clicks them.  No abusive behaviour, no compromise of security, is threatened via getpersonas in the current configuration.

Given those answers, restoring WONTFIX.  If you want to bring it up at a security review, that's your call.  If anyone's tempted to reopen this bug, talk to me first.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

I'm not going to reopen this bug, but I do think it should be fixed, and I'll note the danger that I don't think has been pointed out yet:

Reusing the whitelist for personas allows websites to get onto the whitelist through personas, which are safe to install (at worst, they are an annoyance) and then exploit that status to get the user to install extensions and themes, where are unsafe to install (at worst, they can steal all your personal information and delete all your files from your hard drive).

In other words, it conflates two website behaviors with very different security profiles, applying a permission to do something innocuous to a request to do something very dangerous.

Comment 12

[Justin Scott [:fligtar]]

If you won't accept any of the above concerns, would you consider that it's a huge annoyance for the web developers and services that support Firefox and not all that hard for Firefox to fix?

Comment 13

[Mike Connor [:mconnor]]

(In reply to comment #11)
> Reusing the whitelist for personas allows websites to get onto the whitelist
> through personas, which are safe to install (at worst, they are an annoyance)
> and then exploit that status to get the user to install extensions and themes,
> where are unsafe to install (at worst, they can steal all your personal
> information and delete all your files from your hard drive).

The level of social engineering required here seems more difficult/less useful than simply convincing a user to install a malicious extension.

1. create a site with a bunch of interesting personas
2. convince a user to whitelist the site to "get the full preview experience" or something, involving instructions on going to page info/preferences and changing the whitelist.
3. once the user has the site whitelisted, spam them with dialogs until they relent and install the malicious add-on.

vs.

1. tell a user how to click the Allow button on the infobar to install your cool extension.
or:
1a. have the user copy/paste the link into the URL bar.

Remember, it's _one_ click to allow the modal install window for an add-on.  Anything more complicated is going to be less effective for conversion.  If you're convincing a user to do something, why would you do something so elaborate, merely to abuse the whitelist?

(In reply to comment #12)
> If you won't accept any of the above concerns, would you consider that it's a
> huge annoyance for the web developers and services that support Firefox and not
> all that hard for Firefox to fix?

It's a huge annoyance why?  It may be trivial to use a different whitelist for the personas check, but given that the UI doesn't differentiate between themes and lightweight themes anywhere else, it introduces some very difficult UI issues.  This is basically forcing the user to understand an implementation detail between the theme types, and forcing that on users is a cop-out, and I won't do it unless I am convinced it's a necessary and useful distinction.

Comment 14

[Daniel Veditz [:dveditz]]

It also means that if you whitelist a site for addons (which present a modal confirmation dialog) that site--or an XSS attack on that site--can change your browser chrome suddenly without any warning or confirmation. Yes, it's not actually destructive and can be undone by most users (probably), but it's disturbing.

Comment 15

[Dão Gottwald [:dao]]

(The theme component is for browser/themes/*.)

Comment 16

[Daniel Veditz [:dveditz]]

Disturbing and considered a security problem by some of our users. You can argue and call them stupid, but they're still going to look for a "safer" browser. For example:
http://keyboardcowboy.ca/2010/03/serious-security-flaw-in-personas-for-firefox/

Comment 17

[Dão Gottwald [:dao]]

(In reply to comment #16)
> Disturbing and considered a security problem by some of our users. You can
> argue and call them stupid, but they're still going to look for a "safer"
> browser. For example:
> http://keyboardcowboy.ca/2010/03/serious-security-flaw-in-personas-for-firefox/

That user filed bug 554856.

Comment 19

[Mike Kaply [:mkaply]]

We should revisit this with the removal of heavy theming and the creation of WebExtensions.

We should straightforwardly allow sites other than AMO to host/preview/install lightweight themes without forcing xpinstall permission for those sites.

We should show standard permission notifications "This site wants to preview/themes, do you want to allow it" and add it to a custom lightweight theme permission.

Lightweight themes are the one area where we (Mozilla) maintain a completely walled garden. We shouldn't do that.