Michael Krax Reporter
	Description • 20 years ago

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:He2P9xW@www.mikx.de/firewalling/
2. Follow instructions

	Daniel Veditz [:dveditz]
	Comment 1 • 20 years ago

Confirmed.

	Daniel Veditz [:dveditz]
	Comment 2 • 19 years ago

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]

	Daniel Veditz [:dveditz]
	Comment 3 • 19 years ago

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.

	Mike Connor [:mconnor] Assignee
	Comment 4 • 19 years ago

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.

	Mike Connor [:mconnor] Assignee
	Comment 5 • 19 years ago

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.

	Mike Connor [:mconnor] Assignee
	Comment 6 • 19 years ago

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.

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 7 • 19 years ago

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?

	Boris Zbarsky [:bzbarsky]
	Comment 8 • 19 years ago

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:.

	Mike Connor [:mconnor] Assignee
	Comment 9 • 19 years ago

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.

	Christian :Biesinger (don't email me, ping me on IRC)
	Comment 10 • 19 years ago

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

	Mike Connor [:mconnor] Assignee
	Comment 11 • 19 years ago

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

	Mike Connor [:mconnor] Assignee
	Comment 12 • 19 years ago

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.

	Daniel Veditz [:dveditz]
	Comment 13 • 19 years ago

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

r=dveditz

	Jay Patel [:jay]
	Comment 14 • 19 years ago

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

lets get this checked in. a=jay

	Mike Connor [:mconnor] Assignee
	Comment 15 • 19 years ago

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).

	Mike Connor [:mconnor] Assignee
	Comment 16 • 19 years ago

Landed on branch, waiting on trunk approval.

	Daniel Veditz [:dveditz]
	Comment 17 • 19 years ago

Please land on the trunk

	Mike Connor [:mconnor] Assignee
	Comment 19 • 19 years ago

fixed

	Jay Patel [:jay]
	Comment 20 • 19 years ago

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.

	Daniel Veditz [:dveditz]
	Comment 21 • 19 years ago

Adding distributors

	Daniel Veditz [:dveditz]
	Comment 22 • 19 years ago

Security advisories published