527463 - Update checks for lightweight themes should not happen for non-whitelisted sites

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[Johnathan Nightingale [:johnath]]

See discussion in bug 520346 - updateURLs could be constructed by malicious sites to contain unique identifiers, and thereby enable (albeit low-resolution) tracking of individuals. We should only respect update URLs that point to whitelisted sites.

Comment 1

[Daniel Veditz [:dveditz]]

And possibly only update urls that are same-origin with where you got the theme originally, though I suppose that's an independent question.

Comment 2

[Dave Townsend [:mossop] (PTO until Aug 21st)]

The alternate view is that we only respect update urls for personas installed from whitelisted sites or restrict it more so the persona had to have been installed from a whitelisted site and can only point to a whitelisted site.

Comment 3

[Johnathan Nightingale [:johnath]]

(In reply to comment #2)
> The alternate view is that we only respect update urls for personas installed
> from whitelisted sites or restrict it more so the persona had to have been
> installed from a whitelisted site and can only point to a whitelisted site.

Doing so would mean storing an extra piece of metadata about the persona, right (i.e. from whence it was installed?) Does it get us extra security? If a third party site convinces a victim to install a persona with a getpersonas.com/AMO updateURL, is there anything sneaky that can be done? I don't *think* so, but I'd welcome counterexamples.

In the absence of an unseen risk there, I'm thinking that just checking the updateURL against the whitelist before updating would be simple and effectively plug the hole?

Again, I don't mean these to be loaded sentences; if there's a reason not to do it this way, let's work through it.

[The flip side is that binding it to install-time permission theoretically lets an AMO-hosted persona use a third party update URL which isn't immediately bad, but doesn't feel like a use case we should try hard to enable anyhow, since it confused trust relationships somewhat.]

Comment 4

[Dave Townsend [:mossop] (PTO until Aug 21st)]

(In reply to comment #3)
> (In reply to comment #2)
> > The alternate view is that we only respect update urls for personas installed
> > from whitelisted sites or restrict it more so the persona had to have been
> > installed from a whitelisted site and can only point to a whitelisted site.
> 
> Doing so would mean storing an extra piece of metadata about the persona, right
> (i.e. from whence it was installed?) Does it get us extra security? If a third
> party site convinces a victim to install a persona with a getpersonas.com/AMO
> updateURL, is there anything sneaky that can be done? I don't *think* so, but
> I'd welcome counterexamples.

Well no, we'd just drop the updateURL property when installing something from a non-whitelisted site. I grant you though that maybe they can't do anything bad there.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Notes from a quick chat w/Shaver:

 - not any different from search plugins
 - we should try to isolate cookies
 - doesn't think it blocks

Comment 6

[Daniel Veditz [:dveditz]]

- search plugins don't appear to update in practice (neither the ones we ship nor any of the handful I've managed to collect)
- by default they would update check every 7 days not every day
- they are limited to update from https: urls

Comment 7

[Mike Shaver (:shaver emeritus)]

Do search plugins not check for an update, or do we just not have updates published for them?  I'm pretty sure that we have used the update mechanism in the past (around the transition to Yahoo for CKJT locales, f.e.).

I don't think the concern here is interception of persona updates -- though that's perhaps a separate useful concern to discuss in another bug, just because it's not really related to the whitelistedness of a site -- but rather the phone-home that tells the persona provider that you are still using the persona.

I think the same-origin restriction, and indeed the whitelist restriction, will be onerous even for our own use, since it limits our ability to segregate update traffic to other hostnames.

Comment 8

[Daniel Veditz [:dveditz]]

We still appear to have search update code and related prefs; I assume it's still functional. None of the searchplugins we ship contain an updateURL (I also searched the l10n repo), and anecdotally the few I've added myself from or for major sites don't use it either.

> the phone-home that tells the persona provider that you are still using
> the persona.

It doesn't necessarily do that, though:
 1) Getpersonas.com hosts a theme
 2) Someone Diggs/Tweets a theme install. Might have a different ID.
 3) updateURL points at <evil adtracker of your choice>

Maybe part of the problem is that there is no such thing as "a persona", it's just a collection of metadata passed in programmatically from a webpage. If instead a persona was a JSON or XML file that got loaded from a server we could do sensible things like require the updateURL have to go back to the server you got the persona from. Failing that we could require that the updateURL and the imageURLs all have to come from the same site.

Of course that could still be used by the <evil adtracker> through a redirect.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #6)
> - search plugins don't appear to update in practice (neither the ones we ship
> nor any of the handful I've managed to collect)

Yes, use of the system is uncommon. We still fully support it, though, and intend to use it more frequently once bug 511017 lands.

> - by default they would update check every 7 days not every day

This interval can be specified by the plugin and can be as low as 1 day.

> - they are limited to update from https: urls

This restriction only applies to the engines we ship by default (or that are shipped in extensions). Engines installed from the web can update from non-https URLs.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

The more likely path to a fix here is just disabling third party install of personas, I think.

Comment 11

[Thorin [:thorin]]

Cleaning up open/old privacy tagged bugs .. lightweight themes were removed in FF68+, see Bug 1525762 (part 3b), I think we can close this as INVALID

pinging Dão :)