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

RESOLVED FIXED

Status

()

Firefox
Menus
--
major
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Michael Krax, Assigned: mconnor)

Tracking

({fixed-aviary1.0.5})

unspecified
fixed-aviary1.0.5
Points:
---
Bug Flags:
blocking-aviary1.0.5 +
blocking1.8b3 +
blocking-aviary1.5 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix], URL)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

12 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
Confirmed.
Assignee: nobody → dveditz
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b3+
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.4+
Whiteboard: [sg:fix]
Blocks: 256195
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]
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.

Updated

12 years ago
Whiteboard: [sg:fix] → [sg:fix] find and port trunk patch to 1.0.5
(Assignee)

Comment 4

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

Assignee: dveditz → mconnor
(Assignee)

Comment 5

12 years ago
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.
Attachment #186449 - Flags: review?(dveditz)
(Assignee)

Comment 6

12 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.
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?
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:.
(Assignee)

Updated

12 years ago
Attachment #186449 - Attachment is obsolete: true
Attachment #186449 - Flags: review?(dveditz)
(Assignee)

Comment 9

12 years ago
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.
(Assignee)

Updated

12 years ago
Attachment #186566 - Flags: review?(dveditz)
that won't help from a security point of view, right? because a javascript: url
may well refer to a valid image.
(Assignee)

Comment 11

12 years ago
I need a better testcase that embeds malcious script in a valid javascript
image, this blocks the testcase here, obviously.
Status: NEW → ASSIGNED
(Assignee)

Comment 12

12 years ago
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.
Attachment #186595 - Flags: review?(dveditz)
Comment on attachment 186595 [details] [diff] [review]
defence in depth (alternate patch)

r=dveditz
Attachment #186595 - Flags: review?(dveditz) → review+
(Assignee)

Updated

12 years ago
Attachment #186566 - Attachment is obsolete: true
Attachment #186566 - Flags: review?(dveditz)
(Assignee)

Updated

12 years ago
Attachment #186595 - Flags: approval-aviary1.0.5?

Comment 14

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

lets get this checked in. a=jay
Attachment #186595 - Flags: approval-aviary1.0.5? → approval-aviary1.0.5+

Updated

12 years ago
Whiteboard: [sg:fix] find and port trunk patch to 1.0.5 → [sg:fix] need landing
(Assignee)

Comment 15

12 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).
(Assignee)

Updated

12 years ago
Attachment #186595 - Flags: approval-aviary1.1a2?
(Assignee)

Comment 16

12 years ago
Landed on branch, waiting on trunk approval.
Keywords: fixed-aviary1.0.5

Updated

12 years ago
Attachment #186595 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Please land on the trunk
Whiteboard: [sg:fix] need landing → [sg:fix] need trunk anding
(Assignee)

Comment 18

12 years ago
Created attachment 187176 [details] [diff] [review]
trunk version (post-OS X shellservice landing)
(Assignee)

Comment 19

12 years ago
fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: [sg:fix] need trunk anding → [sg:fix]

Comment 20

12 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.
Adding distributors
Security advisories published
Group: security

Updated

12 years ago
Flags: testcase+

Updated

10 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.