292737 - "Set As Wallpaper" context menu dialog allows to execute arbitrary code

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[Michael Krax]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3

The "Set As Wallpaper" dialog takes the image url as a parameter without
validating it. This allows to execute javascript in chrome and to run arbitrary
code.

By using absolute positioning and the moz-opacity filter an attacker can easily
fool the user to think he is setting a valid image as wallpaper. 

Reproducible: Always

Steps to Reproduce:
1. Open http://bugzilla:[e-mail]/firewalling/
2. Follow instructions

Comment 1

[Daniel Veditz [:dveditz]]

Confirmed.

Comment 2

[Daniel Veditz [:dveditz]]

This doesn't work in the Deer Park alpha, but 1.0.4 still vulnerable. Worth
checking out what changed on the trunk. The setWallpaper.xul code all looks the
same, and the linenumber for the exception I see from browser.js doesn't make a
lot of sense (middle of the toggleSidebar() method? no nsIURI.host in sight).

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIURI.host]"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js
:: anonymous :: line 3704"  data: no]

Comment 3

[Daniel Veditz [:dveditz]]

Oh right, conditional processing of chrome files... in the source it's really
line 3918, in initMiscItems() -- makes a lot more sense. Same code exists on the
1.0 branch, though, and the exception doesn't fire there. this.onImage must be
true or the set as wallpaper menu item wouldn't be made visible.

The exception fires as the trunk is creating the context menu, before you click
on the Set as wallpaper item.

Comment 4

[Mike Connor [:mconnor]]

So, the exception that was getting thrown here isn't getting thrown now that the
bug causing it was fixed, so I can reproduce on trunk as well.

Comment 5

[Mike Connor [:mconnor]]

Attachment: patch (stopgap)

This filters on uri.scheme data/javascript to block this.onImage in building
the context menu.  This is sort of a stopgap in that it breaks a potential
legitimate use, but I haven't seen anything wrt data: or javascript: images
that people would use as wallpaper.  I don't know if there's a more reliable
way to detect a valid image that's not hacky or a bigger change than this.

Comment 6

[Mike Connor [:mconnor]]

I'm sure there's a better fix for this, but I can't think of it at 3 AM.  Even
if we just take this for branch and use something new on trunk for 1.8b3, that's
ok by me.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

will this work in all cases? what about wyciwyg (ok, that may not be an issue
here) or jar? maybe this should use the scriptsecuritymanager?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Why are we filtering out data: here, exactly?  The issue here is that opening he
channel to save the image executes script, no?  That's not a problem with data:.

Comment 9

[Mike Connor [:mconnor]]

Attachment: defence in depth

basically, in this case we should be ensuring (beyond security considerations)
that all broken images don't get here.	So this patch disables the context menu
item if the image hasn't loaded, and throws another check in setWallpaper.xul
to ensure that if there's another caller floating around in extension-land that
we're still safe.  Tested the hell out of this, and its ready to go without
breaking legit uses afaict.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

that won't help from a security point of view, right? because a javascript: url
may well refer to a valid image.

Comment 11

[Mike Connor [:mconnor]]

I need a better testcase that embeds malcious script in a valid javascript
image, this blocks the testcase here, obviously.

Comment 12

[Mike Connor [:mconnor]]

Attachment: defence in depth (alternate patch)

block javascript URIs outright, same as favicons.  I don't know if we want to
break potentially legitimate usage, but this really really blocks the exploit.

Comment 13

[Daniel Veditz [:dveditz]]

Comment on attachment 186595 [details] [diff] [review]
defence in depth (alternate patch)

r=dveditz

Comment 14

[Jay Patel [:jay]]

Comment on attachment 186595 [details] [diff] [review]
defence in depth (alternate patch)

lets get this checked in. a=jay

Comment 15

[Mike Connor [:mconnor]]

If someone can land this in time for nightly builds, I don't have my private key
installed here (need to get this thing running and building tomorrow now that
I'm in NoCal).

Comment 16

[Mike Connor [:mconnor]]

Landed on branch, waiting on trunk approval.

Comment 17

[Daniel Veditz [:dveditz]]

Please land on the trunk

Comment 18

[Mike Connor [:mconnor]]

Attachment: trunk version (post-OS X shellservice landing)

Comment 19

[Mike Connor [:mconnor]]

fixed

Comment 20

[Jay Patel [:jay]]

v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9)
Gecko/20050706 Firefox/1.0.5 using the mikx testcase. The "Set As Wallpaper"
menu item is now disabled.

Comment 21

[Daniel Veditz [:dveditz]]

Adding distributors

Comment 22

[Daniel Veditz [:dveditz]]

Security advisories published