Last Comment Bug 292737 - "Set As Wallpaper" context menu dialog allows to execute arbitrary code
: "Set As Wallpaper" context menu dialog allows to execute arbitrary code
: fixed-aviary1.0.5
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: unspecified
: All All
: -- major (vote)
: ---
Assigned To: Mike Connor [:mconnor]
: Jared Wein [:jaws] (please needinfo? me)
Depends on:
Blocks: sbb?
  Show dependency treegraph
Reported: 2005-05-03 07:17 PDT by Michael Krax
Modified: 2007-04-01 15:02 PDT (History)
7 users (show)
dveditz: blocking‑aviary1.0.5+
dveditz: blocking1.8b3+
dveditz: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (stopgap) (1.42 KB, patch)
2005-06-15 23:53 PDT, Mike Connor [:mconnor]
no flags Details | Diff | Splinter Review
defence in depth (2.66 KB, patch)
2005-06-16 23:26 PDT, Mike Connor [:mconnor]
no flags Details | Diff | Splinter Review
defence in depth (alternate patch) (3.27 KB, patch)
2005-06-17 07:08 PDT, Mike Connor [:mconnor]
dveditz: review+
jaymoz: approval‑aviary1.0.5+
asa: approval‑aviary1.1a2+
Details | Diff | Splinter Review
trunk version (post-OS X shellservice landing) (3.99 KB, patch)
2005-06-23 18:40 PDT, Mike Connor [:mconnor]
no flags Details | Diff | Splinter Review

Description Michael Krax 2005-05-03 07:17:09 PDT
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

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
2. Follow instructions
Comment 1 Daniel Veditz [:dveditz] 2005-05-03 11:35:15 PDT
Comment 2 Daniel Veditz [:dveditz] 2005-06-01 17:56:39 PDT
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 in sight).

Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) []"  nsresult: "0x80004005
(NS_ERROR_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js
:: anonymous :: line 3704"  data: no]
Comment 3 Daniel Veditz [:dveditz] 2005-06-01 18:08:44 PDT
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] 2005-06-15 22:41:59 PDT
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] 2005-06-15 23:53:21 PDT
Created attachment 186449 [details] [diff] [review]
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] 2005-06-15 23:57:42 PDT
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) 2005-06-16 14:12:18 PDT
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 [:bz] (still a bit busy) 2005-06-16 15:05:20 PDT
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] 2005-06-16 23:26:25 PDT
Created attachment 186566 [details] [diff] [review]
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) 2005-06-17 05:42:48 PDT
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] 2005-06-17 06:43:41 PDT
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] 2005-06-17 07:08:40 PDT
Created attachment 186595 [details] [diff] [review]
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] 2005-06-19 23:46:36 PDT
Comment on attachment 186595 [details] [diff] [review]
defence in depth (alternate patch)

Comment 14 Jay Patel [:jay] 2005-06-20 00:02:43 PDT
Comment on attachment 186595 [details] [diff] [review]
defence in depth (alternate patch)

lets get this checked in. a=jay
Comment 15 Mike Connor [:mconnor] 2005-06-20 23:40:43 PDT
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] 2005-06-21 14:43:01 PDT
Landed on branch, waiting on trunk approval.
Comment 17 Daniel Veditz [:dveditz] 2005-06-23 11:45:57 PDT
Please land on the trunk
Comment 18 Mike Connor [:mconnor] 2005-06-23 18:40:06 PDT
Created attachment 187176 [details] [diff] [review]
trunk version (post-OS X shellservice landing)
Comment 19 Mike Connor [:mconnor] 2005-06-23 18:41:15 PDT
Comment 20 Jay Patel [:jay] 2005-07-06 17:02:35 PDT
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] 2005-07-12 11:34:39 PDT
Adding distributors
Comment 22 Daniel Veditz [:dveditz] 2005-07-12 18:05:40 PDT
Security advisories published

Note You need to log in before you can comment on or make changes to this bug.