Note: There are a few cases of duplicates in user autocompletion which are being worked on.

The view-source: pseudo protocol can be used to do cross-domain scripting

RESOLVED FIXED in mozilla1.8beta2

Status

()

Core
Security
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: Michael Krax, Assigned: Darin Fisher)

Tracking

({fixed-aviary1.0.4, fixed1.7.8})

Trunk
mozilla1.8beta2
fixed-aviary1.0.4, fixed1.7.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.8 +
blocking-aviary1.0.4 +
blocking-aviary1.5 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix]DUPEME, 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 view-source: pseudo protocol can be used to do cross-domain scripting
without user interaction. This can be used to steal cookies from other hosts or
manipulate the DOM cross-domain. 

The demo will show the given URL in the IFRAME and will raise an alert box
containing the document.location and document.cookie property of that URL when
the page is completly loaded. An attacker could easily automate that action and
send the data silently to another host. 

Reproducible: Always

Steps to Reproduce:
1. Open http://bugzilla:Ue9wKL2@www.mikx.de/fireviewing/
2. Follow instructions
Confirming.
Blocks: 256195
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:fix]
Created attachment 181225 [details] [diff] [review]
untested patch (attached to the wrong bug)
I think this patch is on the wrong bug. It does not fix this bug, but does fix
"firelinking2" bug 290949. r=dveditz for that one.
Attachment #181225 - Attachment is obsolete: true
Flags: blocking1.7.8+
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.4+
Worth noting that I see the bug on the Aviary 1.0.1 branch, but on the trunk I get:

Error: uncaught exception: Permission denied to get property Window.alert
Attachment #181225 - Attachment description: untested patch → untested patch (attached to the wrong bug)

Comment 5

12 years ago
We have existing bugs on the fact that view-source:javascript: runs JS at all.
Whiteboard: [sg:fix] → [sg:fix]DUPEME
Created attachment 182606 [details] [diff] [review]
patch that fixes this

This fixes this bug (and probably a bunch of the other related ones) by
removing what seems to me like rather bogus error handling.  The question is: 
what does this patch break?
(Note that if caps had a notion of 'not a principal' (i.e., something that fails
all tests, including cross-domain tests with itself), we could use that here.)
FWIW, it seems like that code was added in bug 31818.
Some of the comments there suggest that caps was then returning something more
like 'not a principal'.
(In reply to comment #5)
> We have existing bugs on the fact that view-source:javascript: runs JS at all.

bug 204779 was the one I found, and people argued against fixing it.
Depends on: 204779
(In reply to comment #6)
> This fixes this bug (and probably a bunch of the other related ones) by
> removing what seems to me like rather bogus error handling.  The question is: 
> what does this patch break?

Yeah, good question. I ran with this change and with a breakpoint set in this
else case and never hit it, I ran JS bookmarklets and typed javascript: URLs in
the URL bar, clicked on javascript: links etc. So I don't know what breaks, if
anything at all.

Comment 12

12 years ago
> and people argued against fixing it.

I think people just pointed out that that bug would be fixed automagically if
wyciwyg happened for javascript: content.  And if that happened, we could
disable view-source:javascript: altogether with no loss, imo.

cc:ing caillon, in case he has thoughts on comment 7.
Comment on attachment 182606 [details] [diff] [review]
patch that fixes this

r- after much discussion. Although this fixes *this* case, there are in fact
other cases where there won't be an owner. For example, javascript: urls as the
src for style-sheets or images. Don't laugh, there are actually tutorial pages
on the web that tell people how to generate images (particularly .xbm) from
javascript. javascript: style sheets work in IE and Opera (but not Safari and
Konqueror).

Going to try the approach in bug 204779, although it does kill some useful
functionality. Note also this applies to jar:javascript: urls, which might be
semi-useful, but could be more reasonably replaced with jar:data: urls (for
very small archives)

See https://bugzilla.mozilla.org/attachment.cgi?id=183210 for an example of a
working jar:javascript url.
Attachment #182606 - Flags: review-
(Assignee)

Comment 15

12 years ago
Created attachment 183232 [details] [diff] [review]
v2 patch

This patch is my patch from bug 204779 plus similar changes to nsJARChannel.cpp


Blocking view-source:javascript: makes sense since evaluation of the URL would
anyways run in the wrong window context.  So, it isn't really that useful
today.	Fixing it properly, leveraging wyciwyg is probably the best choice
long-term.
Assignee: dveditz → darin
Status: NEW → ASSIGNED
Attachment #183232 - Flags: superreview?(jst)
Attachment #183232 - Flags: review?(dveditz)
(Assignee)

Updated

12 years ago
Target Milestone: --- → mozilla1.8beta2
Comment on attachment 183232 [details] [diff] [review]
v2 patch

sr=jst
Attachment #183232 - Flags: superreview?(jst) → superreview+
Attachment #182606 - Attachment is obsolete: true
Comment on attachment 183232 [details] [diff] [review]
v2 patch

r=dveditz
Attachment #183232 - Flags: review?(dveditz)
Attachment #183232 - Flags: review+
Attachment #183232 - Flags: approval1.8b2?
Attachment #183232 - Flags: approval1.7.8?
Attachment #183232 - Flags: approval-aviary1.0.4?
Attachment #183232 - Flags: approval1.8b2?
Attachment #183232 - Flags: approval1.8b2+
Attachment #183232 - Flags: approval1.7.8?
Attachment #183232 - Flags: approval1.7.8+
Attachment #183232 - Flags: approval-aviary1.0.4?
Attachment #183232 - Flags: approval-aviary1.0.4+
(Assignee)

Comment 18

12 years ago
Created attachment 183234 [details] [diff] [review]
v2 patch (for moz1.7 and aviary1.0.1 branch)

Here's the patch that I ended up checking in on the branches.  I could not use
LowerCaseEqualsLiteral on the branches.
(Assignee)

Comment 19

12 years ago
fixed-aviary1.0.4 and fixed1.7.8

dbaron suggested that i wait to land this on the trunk after 1.0.4 is out.  so,
i have not landed it on the trunk yet.
Keywords: fixed-aviary1.0.4, fixed1.7.8

Comment 20

12 years ago
http://mozilla.osuosl.org/pub/mozilla.org/firefox/nightly/2005-05-10-22-aviary1.0.1/firefox-1.0.4.en-US.win32.zip
is still vulnerable to the jar variation of bug 292691 (in attachment 183210 [details]). 
Wasn't this patch supposed to fix that variation?

Comment 21

12 years ago
Hmm, nsJARChannel.cpp is present in both modules/libjar/ and
netwerk/protocol/jar/src/ but only copy in modules/libjar/ was patched.
Shouldn't both files be patched (and kept in sync) ?
urg, branches use the netwerk/protocol/jar version (which is gone on trunk). Why
does nsJARChannel exist in modules/libjar?
(Assignee)

Comment 23

12 years ago
jar protocol changes applied to the version under netwerk/protocol/jar/src

damn!  how did the branches end up with both versions?  that was very confusing.
 my bad for only testing my trunk build w/ this patch.
*** Bug 291618 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 25

12 years ago
I checked in the v2 patch on the trunk.

Marking FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Clearing security flag from announced vulnerabilities fixed in Firefox
1.0.4/Mozilla 1.7.8
Group: security

Comment 27

12 years ago
The security flag seems to have been left set on bug 291618 (comment #24 says
that bug 291618 is a duplicate of this one) despite the security flag being
cleared on this bug.

Should the security flag on bug 291618 have also been cleared?
Flags: blocking-aviary1.0.5+ → blocking-aviary1.0.4+
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.