290036 - Link tag allows to execute arbitrary code without user interaction

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[Michael Krax]

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

The link tag allows to load a custom image as the icon for a website, displayed
in the location bar and in the tab title. By setting the href attribute of this
tag to a javascript url, it is possible to call chrome functions and run
arbitrary code without user interaction.



Reproducible: Always

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




The example is cross platform: On Windows this example creates the file
c:\booom.bat and launches it (opens a dos box with a dir command). On Linux
(tested Fedora Core) and MacOSX the example creates the file ~/booom.txt or
/booom.txt. 

The non-windows examples are only roughly tested. Please don't complain if not
working. The way i need to double encode the backslash on linux looks a little
fishy and i doubt every Mac user can write to root by default. You get full user
rights with UniversalXPConnect, so everything else is just a matter of
implementation time.

Comment 1

[Daniel Veditz [:dveditz]]

Confirming. Looks like it would work in the Suite, too, with a little quote
tweaking -- it got as far as the delayedOpenWindow() call so it's already
running with chrome privs.

Comment 2

[David Baron :dbaron:]

Attachment: possible patch

I think this should fix it, but the testcase gives an error on Linux, and I
think the error is very early in the process:

Error: illegal character
Source File:
javascript:netscape.security.PrivilegeManager.enablePrivilege(\'UniversalXPConnect\');file=Components.classes[\'@mozilla.org/file/local;1\'].createInstance(Components.interfaces.nsILocalFile);file.initWithPath(\'~/booom.txt\');file.createUnique(Components.interfaces.nsIFile.NORMAL_FILE_TYPE,420);outputStream=Components.classes[\'@mozilla.org/network/file-output-stream;1\'].createInstance(Components.interfaces.nsIFileOutputStream);outputStream.init(file,0x04|0x08|0x20,420,0);output=\'booom!\';outputStream.write(output,output.length);outputStream.close();

Line: 1, Column: 51
Source Code:
netscape.security.PrivilegeManager.enablePrivilege(\'UniversalXPConnect\');file=Components.classes[\'@mozilla.org/file/local;1\'].createInstance(Components.interfaces.nsILocalFile);file.initWithPath(\'~/booom.txt\');file.createUnique(Components.interfaces



If somebody could try this on Windows that would be nice.

Comment 3

[David Baron :dbaron:]

Comment on attachment 180499 [details] [diff] [review]
possible patch

This doesn't actually work; I fixed the testcase by making the Linux line look
like the others (changing "\\\'" to "\'").

Comment 4

[Michael Krax]

I knew this \\\' was fishy. It turned out i tested linux on a really old 0.9
build that required that encoding (for whatever reason). Tested again with a 1.0
and it requires a \' only. Changed the PoC accordingly.

Comment 5

[David Baron :dbaron:]

Comment on attachment 180499 [details] [diff] [review]
possible patch

I also had a zombie firefox process running earlier.  This patch works fine.

Comment 6

[David Baron :dbaron:]

Attachment: alternative patch

We really shouldn't block data:, though, since it's useful for favicons.

Comment 7

[Brendan Eich [:brendan]]

Comment on attachment 180541 [details] [diff] [review]
alternative patch

sr=me if you can prove we're not vulnerable to data: URLs encoding script (in a
data: text/html URL, or otherwise).

/be

Comment 8

[David Baron :dbaron:]

Comment on attachment 180499 [details] [diff] [review]
possible patch

Neither dveditz nor I have been able to write an exploit using data: URLs, so I
think the alternative patch is ok.

Comment 9

[David Baron :dbaron:]

Attachment: patch for 1.7 branch

A bunch of stuff wasn't on the 1.7 branch; this merges it.

Comment 10

[David Baron :dbaron:]

Attachment: patch for 1.7 branch

Better merge.

Comment 11

[Chase Phillips]

Comment on attachment 180541 [details] [diff] [review]
alternative patch

a=chase pending dbaron's review

Comment 12

[Chase Phillips]

Moving to Core.

Comment 13

[Chase Phillips]

blocking1.7.7+

Comment 14

[Chase Phillips]

Comment on attachment 180545 [details] [diff] [review]
patch for 1.7 branch

assuming this is the patch you want to go with, a=chase pending r/sr

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 180541 [details] [diff] [review]
alternative patch

r=dveditz
I was worried about data: urls, but after a bunch of debugging it looks like
this is never used in a context that would parse it as html and run scripts.
I'm still a little nervous

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 180545 [details] [diff] [review]
patch for 1.7 branch

r=dveditz
carrying over sr on the grounds that this makes 1.7 match already-reviewed
aviary.

Comment 17

[David Baron :dbaron:]

Fixed on trunk.

Comment 18

[Daniel Veditz [:dveditz]]

Fix released