290949 - Link tag still allows to execute arbitrary code without user interaction (with view-source:javascript: URL)

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/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

By setting the href attribute of a link tag to a view-source:javascript url, it
is possible to call chrome functions and run arbitrary code without user
interaction (this breaks the fix for #290036).

Reproducible: Always

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




The patch for #290036 only added the following blocker:

+ if (uri.schemeIs("javascript"))
+ return;

By adding view-source: in front the script can pass this check and the
javascript code gets executed.

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. Depending on caching the the script might run twice in some cases
(this will create an additional booom-1.txt).

The non-windows examples are only roughly tested. Please don't complain if not
working. 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]]

attachment 181225 [details] [diff] [review] in bug 290982 was intended for this bug I think.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: untested patch

right

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 181355 [details] [diff] [review]
untested patch

r=dveditz

Comment 4

[Asa Dotzler [:asa]]

Does this need additional review or are we ready to land here? Does this have to
make 1.8b2? If so, it needs to land ASAP.

Comment 5

[Daniel Veditz [:dveditz]]

Logistical issue: we can't ship a 1.0.4 and the 1.1 alpha at the same time, and
the alpha will be shipped as soon as we get the beefed up fixes for the general
class of bugs reported as mfsa2005-41 (bug 289074 et al, fixed on trunk as
281988). I just cc'd mikx on that one.

soooo, if we check this fix in on the trunk a week or two before we can get
1.0.4 out will that expose this bug? It really just looks like a cleanup of the
previous fix and could be checked in with comments to that effect. There's no
obvious pointer to view-source (or jar: as in bug 291150).

Or we ship a vulnerable alpha (to be called "Deer Park", not Firefox-something,
to hopefully prevent massive adoption) and then when 1.0.4 goes out the alpha
people learn about this. Most of them should be willing to grab the next nightly.

Comment 6

[Michael Krax]

(In reply to comment #5)
> soooo, if we check this fix in on the trunk a week or two before we can get
> 1.0.4 out will that expose this bug? 

A public checkin is a more or less a full disclosure of information and
everything beyond a checkin without an advisory is just security by obscurity
from my point of voew (we had that discussion several times already *g*). 

Even if the code does not relate directly to view-source, a change of a security
bugfix will get attention and make it obvious that the first fix is breakable.
And if a hacker is at the point that he is asking himself "mhhh, how can i get
around a javascript protocol check?", than the answer "maybe use another
protocol" will quickly come to his mind. Actually i think it's just a matter of
time until someone looks at the 1.0.3 fixes and tries to break them even without
knowing they are breakable.

This is a remote code execution bug without user interaction. Beside an easy
exploitable buffer overflow this is the worst scenario you can get into from a
security perpective. 

Release 1.0.4 as soon as possible. At any cost. Even if that means to push back
the 1.1 alpha. This is a situation where the Mozilla Foundation can show their
dedication to preemtive user protection. 

I know this oppionion is probably not capable of winning a majority ;)

So if i have to choose from a the realstic options, i would vote to make an 1.1
alpha without the fix that gets obviously marked as "not for production use" and
then get a secure 1.0.4 after that asap.

Comment 7

[georgi - hopefully not receiving bugspam]

sure mikx owns this bug, but imho it is better to commit the patch to the trunk
with non-intrusive comment so more testing for similar problems can be done.

this also protects users running nightlies.

i was involved in a similar situation with the linux kernel and a bunch of
signedness problems - iirc the patches were publicly commited in the repository
at least a week before the official release of the kernel and the sky didn't
fell down.

mikx: if you are concerned that someone disclose the bug because of the
checkins, please have in mind this bugzilla entry shows when you have reported
the bug.

Comment 8

[Michael Krax]

(In reply to comment #7)
> mikx: if you are concerned that someone disclose the bug because of the
> checkins, please have in mind this bugzilla entry shows when you have reported
> the bug.

I am not scared someone will disclose the bug and take credit. Instead i am
concerned about the security of majority of the userbase and the message of
Mozilla products being a stable and secure platform.

Most of you are probably longer involved in doing security stuff than i am. You
have great "best practice" exprience when it comes to those situations when you
have to choose from options that all have a drawback. 

I am sure MoFo will do whatever they consider best and maybe we can discuss
optimization of the entire process in some other place, since that is probably
far out of the scope of this bug report.

Comment 9

[Darin Fisher]

Comment on attachment 181355 [details] [diff] [review]
untested patch

>Index: caps/idl/nsIScriptSecurityManager.idl

>+    //   WARNING: Support for this value was added in Mozilla 1.7.8 and
>+    //   Firefox 1.0.4.  Use in prior versions WILL BE IGNORED.
>+    const unsigned long DISALLOW_SCRIPT = 1 << 3;

nit: maybe only mention the Gecko version here.


>Index: caps/src/nsScriptSecurityManager.cpp

>+    NS_ENSURE_TRUE(!(aFlags & ~(nsIScriptSecurityManager::DISALLOW_FROM_MAIL |
>+                                nsIScriptSecurityManager::ALLOW_CHROME |
>+                                nsIScriptSecurityManager::DISALLOW_SCRIPT |
>+                                nsIScriptSecurityManager::DISALLOW_SCRIPT_OR_DATA)),
>+                   NS_ERROR_UNEXPECTED);

nit: use NS_ENSURE_FALSE to avoid extra NOT operator?


>+    if (((aFlags & (nsIScriptSecurityManager::DISALLOW_SCRIPT |
>+                    nsIScriptSecurityManager::DISALLOW_SCRIPT_OR_DATA)) &&
>+         targetScheme.Equals("javascript")) ||
>+        ((aFlags & nsIScriptSecurityManager::DISALLOW_SCRIPT_OR_DATA) &&
>+         targetScheme.Equals("data")))
>     {
>        return NS_ERROR_DOM_BAD_URI;
>     }

nit: this might be clearer as two separate |if| blocks.


sr=darin

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 181355 [details] [diff] [review]
untested patch

a=asa

Comment 11

[Boris Zbarsky [:bzbarsky]]

Neil, do we need a siilar change to xpfe (esp. on the 1.7 branch)?

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 181355 [details] [diff] [review]
untested patch

a=caillon for the branches

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I copied the fix to xpfe and landed on aviary and 1.7 branches.

Comment 14

[sairuh (rarely reading bugmail)]

other than the test case in comment 0, are there other areas or things we should
test to ensure that this didn't regress anything? thanks!

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

 1) Make sure favicons still work in tabbrowser. (and the URL bar, why not?)
 2) Test some other security bugs related to javascript: and/or data: URLs and
make sure they didn't regress

Comment 16

[Jay Patel [:jay]]

Testcase http://bugzilla:[e-mail]/firelinking2/ appears to be fixed
with latest Win32 Aviary build (Favicons appear to be working fine also):
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050509
Firefox/1.0.4

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk (although it's no longer a security problem there with
darin's fix, I still prefer this approach).

Comment 18

[Daniel Veditz [:dveditz]]

Clearing security flag from announced vulnerabilities fixed in Firefox
1.0.4/Mozilla 1.7.8