304931 - Installing theme from website allowed, even though website is not whitelisted

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Sokraates]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.7.10) Gecko/20050717 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; de-DE; rv:1.7.10) Gecko/20050717 Firefox/1.0.6

On a non-whitelisted site, clicking on an link, which triggers the installation
of a theme, will prompt a notification asking, whether you want to install the
theme, obviously ignoring the whitelist.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www2.warnerbros.com/batmanbegins/flash/index.html
2. Click on "Downloads"
3. Click on "Firefox Skin"
4. Click on the screenshot
5. Click on "Yes"

Actual Results:  
The theme is installed flawlessly. ... 

Expected Results:  
Instead of asking, whether the the should be installed, a notifier bar should
appear stating, that the website is not allowed to install content.

Comment 1

[Sokraates]

*** Bug 304934 has been marked as a duplicate of this bug. ***

Comment 2

[Sokraates]

*** Bug 304932 has been marked as a duplicate of this bug. ***

Comment 3

[Jo Hermans]

From bug 304934 :
This is *not* a dupe of #304727, since not only does the notifier not appear,
but the installation works even for non-whitelisted sites.

Comment 4

[Jesse Ruderman]

This behavior was introduced in bug 246375, "Themes should not be required to be
whitelisted."

Comment 5

[Sokraates]

Maybe I'm just paranoid, but I think *anything* being installed should need to
be whitelisted first.

Concidering, that the notification asking if you want to install the theme does
not even have a 2-second timer, like the one for installing XPIs, the current
"solution" seems a bad one.

If regular users can live with whitelisting sites for installing XPIs and then
patiently wait to be able to click "Yes", they can live with doing the same with
themes. ... Or maybe they only use addons.mozilla.org, which solves the problem
alltogether. :)

Comment 6

[Frank Wein [:mcsmurf]]

Well, themes have much less ability to screw the browser up (ok they could
deliver bad white graphics, but that's all). Extensions can
delete/read/send/whatever files on your PC and more, Themes are very restricted
at what they can do. Due to that this change was intentional, i suggest WONTFIX.

Comment 7

[Daniel Veditz [:dveditz]]

Attachment: make skin installs honor site whitelisting

Comment 8

[Jesse Ruderman]

Frank, see bug 246375 comment 3 for why malicious themes are almost as dangerous
as malicious extensions.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 199199 [details] [diff] [review]
make skin installs honor site whitelisting

mconnor gave verbal r= earlier today

Comment 10

[Asa Dotzler [:asa]]

This is a policy change as much as a code change. We need to get more people
involved in this. 

Comment 11

[Jesse Ruderman]

In addition to making theme installation subject to whitelisting, I think it
would make sense to use the extension installation dialog for installing themes
(more familiar UI, protection against bug 162020, etc).

Comment 12

[Daniel Veditz [:dveditz]]

(In reply to comment #11)
> I think it would make sense to use the extension installation dialog for
> installing themes (more familiar UI, protection against bug 162020, etc).

That's easy, just get rid of the "if (mChromeType == CHROME_SKIN)" block here:
http://lxr.mozilla.org/mozilla/source/xpinstall/src/nsXPInstallManager.cpp#278

But that should be argued in a different bug I think.

If you're worried only about the delay part, we could change to ConfirmEx() and
pass the nsIPrompt::BUTTON_DELAY_ENABLE flag down in ConfirmChromeInstall()

Comment 13

[Mike Connor [:mconnor]]

The argument (apparently) for exempting themes was that theme installations are
not inherently dangerous, but that doesn't coexist with the idea that
whitelisting was implemented for the purpose of removing an annoyance factor,
not as a security hoop.  If endless looping of install requests aren't okay for
extensions, I don't see this as acceptable for themes.

This is a clarification and proper implementation of the policy, IMO.

Comment 14

[dirio49]

i tried the site on 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20051015
Firefox/1.4.1 ID:2005101504
it appears to be fixed, the theme downloaded fine. but was not compatible with
my version of firefox.

Comment 15

[dirio49]

sorry misspoke.
not fixed

Comment 16

[Scott MacGregor]

Dan, can this patch land on the trunk and branch now? Looks like the reviews are
all in place and the bug has approval. Thanks!

Comment 17

[Daniel Veditz [:dveditz]]

fix checked into trunk and 1.8 branch