An image that sends a "Location: javascript:document.location.href='....';" http header will redirect the whole browser window

RESOLVED FIXED in mozilla1.4final

Status

()

Core
Networking: HTTP
P3
major
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: Sam Cannell, Assigned: dougt)

Tracking

Trunk
mozilla1.4final
x86
Windows 2000
Points:
---
Bug Flags:
blocking1.4 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

15 years ago
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

15 years ago
while this could be annoying, i don't think there are any security ramifications.
Actually, I can see not supporting JS in Location: headers.  But yes, this is
not a security bug.

Updated

15 years ago
Group: security
->Networking: HTTP
Assignee: asa → darin
Component: Browser-General → Networking: HTTP
QA Contact: asa → httpqa
(Reporter)

Comment 4

15 years ago
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

15 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 6

15 years ago
Likewise with IE6 and Konqueror (In fact Konq spews warnings apout javascript 
redirects to stdout when it tries to open the image)
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

15 years ago
given the timing of this bug report, i wonder if this bug could be a duplicate
of or possibly related to bug 193887.
(Reporter)

Comment 9

15 years ago
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

14 years ago
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

14 years ago
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...
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.4final
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.
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.
svg doesn't yet work in <img>, and theres a bug somewhere detailing the security
concerns this would have.
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.
Group: security

Comment 16

14 years ago
should try to fix for 1.4 final if possible.
Flags: blocking1.4+

Updated

14 years ago
Whiteboard: [ETA: June 12, 2003]

Comment 17

14 years ago
mitch, doug: can one of you pick this up for 1.4 final?  thanks!
(Assignee)

Comment 18

14 years ago
in 1.4b, i get an uncaught exception: permission deniced to get property
Function.href.

I am updating my tree now.  
Assignee: darin → dougt
Status: ASSIGNED → NEW
(Assignee)

Updated

14 years ago
(Assignee)

Comment 19

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

Comment 20

14 years ago
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?  

Whiteboard: [ETA: June 12, 2003]
Your approach sounds good. We can add another "flag" to CheckLoadURI that
excludes javascript: and data: URLs.
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.
Created attachment 124380 [details] [diff] [review]
Patch - disregard the last one
Attachment #124319 - Attachment is obsolete: true
(Assignee)

Comment 24

14 years ago
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.
Attachment #124380 - Attachment is obsolete: true
Comment on attachment 124384 [details] [diff] [review]
patch v.2

I think this will do for now. r=mstoltz.
Attachment #124384 - Flags: review+
(Assignee)

Comment 26

14 years ago
Comment on attachment 124384 [details] [diff] [review]
patch v.2

brendan, can you review this change?
Attachment #124384 - Flags: superreview?(brendan)
Attachment #124384 - Flags: approval1.4?
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
Attachment #124384 - Flags: superreview?(brendan) → superreview+
(Assignee)

Comment 28

14 years ago
mitch, you pick the name.
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
Attachment #124384 - Flags: approval1.4? → approval1.4+
(Assignee)

Comment 30

14 years ago
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

14 years ago
Adding branch fixed1.4 keyword 
Keywords: fixed1.4
(Assignee)

Comment 32

14 years ago
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$ 
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Updated

14 years ago
Blocks: 211999

Comment 33

14 years ago
Is there an updated test case for this bug? The mentioned site is now gone.
QA Contact: httpqa → benc
(Reporter)

Comment 34

14 years ago
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

13 years ago
So if the fix worked the page simply displays w/ a broken image right?
(Reporter)

Comment 36

13 years ago
Yeah - that's right.

On Mozilla 1.3 and below, it redirected the entire browser window to another 
site.

Updated

13 years ago

Comment 37

13 years ago
V, verified 1.4, using Netscape 7.1
Keywords: fixed1.4 → verified1.4
Bugs published on the Known-vulnerabilities page long ago, removing confidential
flag.
Group: security
You need to log in before you can comment on or make changes to this bug.