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

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

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:[e-mail]/fireviewing/
2. Follow instructions

Comment 1

[Daniel Veditz [:dveditz]]

Confirming.

Comment 2

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

Attachment: untested patch (attached to the wrong bug)

Comment 3

[Daniel Veditz [:dveditz]]

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.

Comment 4

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

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

We have existing bugs on the fact that view-source:javascript: runs JS at all.

Comment 6

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

Attachment: 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?

Comment 7

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

(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.)

Comment 8

[Johnny Stenback (:jst)]

FWIW, it seems like that code was added in bug 31818.

Comment 9

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

Some of the comments there suggest that caps was then returning something more
like 'not a principal'.

Comment 10

[Daniel Veditz [:dveditz]]

(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.

Comment 11

[Johnny Stenback (:jst)]

(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

[Boris Zbarsky [:bzbarsky]]

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

Comment 13

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

cc:ing caillon, in case he has thoughts on comment 7.

Comment 14

[Daniel Veditz [:dveditz]]

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.

Comment 15

[Darin Fisher]

Attachment: 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.

Comment 16

[Johnny Stenback (:jst)]

Comment on attachment 183232 [details] [diff] [review]
v2 patch

sr=jst

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 183232 [details] [diff] [review]
v2 patch

r=dveditz

Comment 18

[Darin Fisher]

Attachment: 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.

Comment 19

[Darin Fisher]

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.

Comment 20

[Jesse Ruderman]

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

[Frederic Crozat]

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) ?

Comment 22

[Christian :Biesinger (don't email me, ping me on IRC)]

urg, branches use the netwerk/protocol/jar version (which is gone on trunk). Why
does nsJARChannel exist in modules/libjar?

Comment 23

[Darin Fisher]

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.

Comment 24

[Daniel Veditz [:dveditz]]

*** Bug 291618 has been marked as a duplicate of this bug. ***

Comment 25

[Darin Fisher]

I checked in the v2 patch on the trunk.

Marking FIXED

Comment 26

[Daniel Veditz [:dveditz]]

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

Comment 27

[Bruce Robson]

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?