Last Comment Bug 195201 - An image that sends a "Location: javascript:document.location.href='....';" http header will redirect the whole browser window
: An image that sends a "Location: javascript:document.location.href='....';" h...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: Trunk
: x86 Windows 2000
: P3 major (vote)
: mozilla1.4final
Assigned To: Doug Turner (:dougt)
: benc
Mentors:
http://plaz.net.nz/mozilla/
Depends on:
Blocks: 211999
  Show dependency treegraph
 
Reported: 2003-02-27 04:40 PST by Sam Cannell
Modified: 2005-03-01 15:04 PST (History)
15 users (show)
chofmann: blocking1.4+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
CAPS patch (4.72 KB, patch)
2003-05-27 15:56 PDT, Mitchell Stoltz (not reading bugmail)
no flags Details | Diff | Splinter Review
Patch - disregard the last one (1.57 KB, patch)
2003-05-28 13:56 PDT, Mitchell Stoltz (not reading bugmail)
no flags Details | Diff | Splinter Review
patch v.2 (4.08 KB, patch)
2003-05-28 14:11 PDT, Doug Turner (:dougt)
security-bugs: review+
brendan: superreview+
rjesup: approval1.4+
Details | Diff | Splinter Review

Description Sam Cannell 2003-02-27 04:40:29 PST
User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.0; PN)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3b) Gecko/20030226

The page linked to from http://plaz.debian.co.nz/mozilla/ has <img 
src="image.jpg"> .. image.jpg is a php script that sends the http 
header "Location: javascript:document.location.href='http://www.mozilla.org/'".

This causes the browser window to be redirected to the mozilla site.

Reproducible: Always

Steps to Reproduce:
Described above
Actual Results:  
Browser window was redirected to www.mozilla.org

Expected Results:  
Not execute javascript delivered in http headers.

Theme was default.
Computer is a duron 850 with 640mb ram, running Windows 2000.
I'll download the nightly Linux build when I get to work tomorrow, and try it 
with that.

I'm guessing this could cause problems with people posting images to forums 
that redirect browsers to porn sites or similar.
Comment 1 timeless 2003-02-27 09:39:17 PST
while this could be annoying, i don't think there are any security ramifications.
Comment 2 Boris Zbarsky [:bz] 2003-02-27 09:42:23 PST
Actually, I can see not supporting JS in Location: headers.  But yes, this is
not a security bug.
Comment 3 Christopher Hoess (gone) 2003-02-27 14:44:39 PST
->Networking: HTTP
Comment 4 Sam Cannell 2003-02-27 22:28:39 PST
I'm hesitant to agree on that.  

I haven't tested it fully, but given that the document.location code is
executing, is there the possibility that other javascript code could be
executed, modifying the document in the web browser without the user's
knowledge?  For example, changing the action property on a form in the document
and redirecting the submission?
Comment 5 Russell Odom 2003-02-28 01:50:30 PST
OK, I see this (1.3b/Win2K). Confirming. I agree with comment 4; I'm sure that
there are plenty of nasty things someone could do with this. Furthermore, I
can't think of any times where you would legitimately put some Javascript in a
Location header that couldn't be done in a better way.

I've just tried in IE5.5, and it isn't susceptible to this; the test URL just
shows a broken image.
Comment 6 Sam Cannell 2003-02-28 01:57:59 PST
Likewise with IE6 and Konqueror (In fact Konq spews warnings apout javascript 
redirects to stdout when it tries to open the image)
Comment 7 Boris Zbarsky [:bz] 2003-02-28 07:12:08 PST
Yeah, but JS code executing this way is no different from JS code executing any
other way, imo (think linking to an image that's SVG and just has script
embedded in it).
Comment 8 Darin Fisher 2003-02-28 13:49:47 PST
given the timing of this bug report, i wonder if this bug could be a duplicate
of or possibly related to bug 193887.
Comment 9 Sam Cannell 2003-02-28 15:03:11 PST
In response to comment 7:

There's no real issue if you're hosting images yourself, but if you have a site
that links to images on a host that you're not in control of, then somebody with
access to the images you're linking could manipulate the content of your site at
the browser.
Comment 10 Jesse Ruderman 2003-05-16 20:34:05 PDT
Many forums allow posts to include <img>s with off-site srces.  I disagree with
bz's conclusion that this isn't a security problem.
Comment 11 Darin Fisher 2003-05-16 21:29:24 PDT
we have discussed possibly limiting HTTP redirects to a whitelist. i don't know
if now is the right time to do that or not...
Comment 12 Bradley Baetz (:bbaetz) 2003-05-16 21:37:46 PDT
I thought that we'd blocked js redirects a long time back.

Even ignoring this, why is the document getting redirected, not the image? Is
this because ofhte nsImageDocument hacks? Even so, 'document' shouldn't be the
parent document.
Comment 13 Boris Zbarsky [:bz] 2003-05-17 00:16:50 PDT
The script is executed in the context of the parent document, in this case.

Jesse, your concern about off-site images does not address the fact that SVG has
precisely that problem, and that as people move from the deprecated (in XHTML)
<img> to <object> this will only get worse, since HTML could be used instead of
SVG as the script vehicle.
Comment 14 Bradley Baetz (:bbaetz) 2003-05-17 05:39:07 PDT
svg doesn't yet work in <img>, and theres a bug somewhere detailing the security
concerns this would have.
Comment 15 Mitchell Stoltz (not reading bugmail) 2003-05-19 12:46:13 PDT
If an HTML page includes a <SCRIPT SRC="script file from some other host"> tag,
then the script runs with the permissions of the page and can modify or read the
DOM of the page. It's always been that way: including a script from another host
gives anyone with write access on that other host control over your page. The
problem here is that there may be an assumption among page authors that the same
is not true for images. Lots of people include images from other hosts without
realizing this security implication. For that reason, I think this is a real
security issue that should be fixed ASAP.

Marking this bug Security-Sensitive.
Comment 16 chris hofmann 2003-05-22 15:39:14 PDT
should try to fix for 1.4 final if possible.
Comment 17 Darin Fisher 2003-05-23 13:29:23 PDT
mitch, doug: can one of you pick this up for 1.4 final?  thanks!
Comment 18 Doug Turner (:dougt) 2003-05-23 15:20:36 PDT
in 1.4b, i get an uncaught exception: permission deniced to get property
Function.href.

I am updating my tree now.  
Comment 19 Doug Turner (:dougt) 2003-05-23 19:15:30 PDT
yup.  i can repro with a build from this morning.  I hope to look at this over
the 3 day weekend.  If someone has time, it would be a good exercise to reduce
the range of dates where this bug started to occur.
Comment 20 Doug Turner (:dougt) 2003-05-27 11:07:47 PDT
I checked version 1.3, 1.4a, and 1.4b.  I could only reproduce this problem in
1.3.  Mitch we attempt to fix any similar issues between 1.3 and now?  I am not
sure if this is a regression or if this is a hole that we never encountered.

This test case is slightly misleading.  The contents of the image doesn't really
matter in the general case.  You could set up a server that on every image
request, the server returns a redirect as part of the response.  

There is no way to prevent an image request from returning a Location field in
the http response.  So, the root of the problem, i believe, is that we are
honoring URI's that have no business being in a Location field.  This follows
darin comment #11.

Also Mitch, we currently call CheckLoadURI() when processing the redirect.  It
would be nice to continue using this check instead of special casing the code in
http.  How do you suggest that we instruct this method to exclude javascript and
possibly other URL schemes during a redirect check?  

Any suggestions on alternative fixes?  

Comment 21 Mitchell Stoltz (not reading bugmail) 2003-05-27 13:40:45 PDT
Your approach sounds good. We can add another "flag" to CheckLoadURI that
excludes javascript: and data: URLs.
Comment 22 Mitchell Stoltz (not reading bugmail) 2003-05-27 15:56:08 PDT
Created attachment 124319 [details] [diff] [review]
CAPS patch

This patch adds a new option to CheckLoadURI that prevents loading javascript:
and data: URLs. To use it, pass nsIScriptSecurityManager::DISALLOW_JAVASCRIPT
as the third argument to CheckLoadURI.
Comment 23 Mitchell Stoltz (not reading bugmail) 2003-05-28 13:56:17 PDT
Created attachment 124380 [details] [diff] [review]
Patch - disregard the last one
Comment 24 Doug Turner (:dougt) 2003-05-28 14:11:19 PDT
Created attachment 124384 [details] [diff] [review]
patch v.2

Although, I am not convinced that this is the best solution to the problem, it
does fix the potential security hole and it is quite simple.
Comment 25 Mitchell Stoltz (not reading bugmail) 2003-05-28 14:47:38 PDT
Comment on attachment 124384 [details] [diff] [review]
patch v.2

I think this will do for now. r=mstoltz.
Comment 26 Doug Turner (:dougt) 2003-05-28 14:51:51 PDT
Comment on attachment 124384 [details] [diff] [review]
patch v.2

brendan, can you review this change?
Comment 27 Brendan Eich [:brendan] 2003-05-28 15:56:25 PDT
Comment on attachment 124384 [details] [diff] [review]
patch v.2

Call it DISALLOW_SCRIPT to generalize to other languages than javascript, and
to take in the notion of data: URLs.  Maybe DISALLOW_SCRIPT_OR_DATA?  Your
call.

/be
Comment 28 Doug Turner (:dougt) 2003-05-28 16:00:09 PDT
mitch, you pick the name.
Comment 29 [:jesup] on pto until 2016/8/1 Randell Jesup 2003-05-29 08:48:53 PDT
Comment on attachment 124384 [details] [diff] [review]
patch v.2

Feel free to change the define name before checkin, but let's get this in. 
a=rjesup
Comment 30 Doug Turner (:dougt) 2003-05-29 12:30:31 PDT
Checking in caps/idl/nsIScriptSecurityManager.idl;
/cvsroot/mozilla/caps/idl/nsIScriptSecurityManager.idl,v  <-- 
nsIScriptSecurityManager.idl
new revision: 1.54.36.1; previous revision: 1.54
done
Checking in caps/src/nsScriptSecurityManager.cpp;
/cvsroot/mozilla/caps/src/nsScriptSecurityManager.cpp,v  <-- 
nsScriptSecurityManager.cpp
new revision: 1.201.2.1; previous revision: 1.201
done
Checking in netwerk/protocol/http/src/nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <-- 
nsHttpChannel.cpp
new revision: 1.165.2.1; previous revision: 1.165
done

Waiting on the trunk to open to check in there.
Comment 31 Paul Wyskoczka 2003-05-29 13:30:31 PDT
Adding branch fixed1.4 keyword 
Comment 32 Doug Turner (:dougt) 2003-05-29 15:08:41 PDT
fixed on trunk:
Checking in caps/idl/nsIScriptSecurityManager.idl;
/cvsroot/mozilla/caps/idl/nsIScriptSecurityManager.idl,v  <-- 
nsIScriptSecurityManager.idl
new revision: 1.55; previous revision: 1.54
done
Checking in caps/src/nsScriptSecurityManager.cpp;
/cvsroot/mozilla/caps/src/nsScriptSecurityManager.cpp,v  <-- 
nsScriptSecurityManager.cpp
new revision: 1.203; previous revision: 1.202
done
Checking in netwerk/protocol/http/src/nsHttpChannel.cpp;
/cvsroot/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp,v  <-- 
nsHttpChannel.cpp
new revision: 1.166; previous revision: 1.165
done
bash-2.05b$ 
Comment 33 benc 2004-03-15 09:06:37 PST
Is there an updated test case for this bug? The mentioned site is now gone.
Comment 34 Sam Cannell 2004-03-15 10:13:06 PST
I've put it back up at http://plaz.net.nz/mozilla/

Sorry about that - the box I had plaz.net.nz hosted on fell over somewhat messily.

Cheers,

Sam
Comment 35 benc 2004-04-28 17:47:56 PDT
So if the fix worked the page simply displays w/ a broken image right?
Comment 36 Sam Cannell 2004-04-28 17:59:01 PDT
Yeah - that's right.

On Mozilla 1.3 and below, it redirected the entire browser window to another 
site.
Comment 37 benc 2004-05-13 18:04:28 PDT
V, verified 1.4, using Netscape 7.1
Comment 38 Daniel Veditz [:dveditz] 2004-07-20 04:47:52 PDT
Bugs published on the Known-vulnerabilities page long ago, removing confidential
flag.

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