Last Comment Bug 290982 - The view-source: pseudo protocol can be used to do cross-domain scripting
: The view-source: pseudo protocol can be used to do cross-domain scripting
Status: RESOLVED FIXED
[sg:fix]DUPEME
: fixed-aviary1.0.4, fixed1.7.8
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.8beta2
Assigned To: Darin Fisher
:
Mentors:
http://bugzilla:Ue9wKL2@www.mikx.de/f...
: 291618 (view as bug list)
Depends on: 204779
Blocks: sbb?
  Show dependency treegraph
 
Reported: 2005-04-19 09:17 PDT by Michael Krax
Modified: 2007-04-01 14:42 PDT (History)
12 users (show)
dveditz: blocking1.7.8+
dveditz: blocking‑aviary1.0.4+
dveditz: blocking‑aviary1.5+
bob: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
untested patch (attached to the wrong bug) (6.54 KB, patch)
2005-04-19 18:11 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch that fixes this (1.71 KB, patch)
2005-05-04 13:06 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
dveditz: review-
Details | Diff | Splinter Review
v2 patch (2.70 KB, patch)
2005-05-10 19:07 PDT, Darin Fisher
dveditz: review+
jst: superreview+
dbaron: approval‑aviary1.0.4+
dbaron: approval1.7.8+
dbaron: approval1.8b2+
Details | Diff | Splinter Review
v2 patch (for moz1.7 and aviary1.0.1 branch) (2.74 KB, patch)
2005-05-10 19:33 PDT, Darin Fisher
no flags Details | Diff | Splinter Review

Description Michael Krax 2005-04-19 09:17:11 PDT
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
Comment 1 Daniel Veditz [:dveditz] 2005-04-19 17:12:40 PDT
Confirming.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-04-19 18:11:15 PDT
Created attachment 181225 [details] [diff] [review]
untested patch (attached to the wrong bug)
Comment 3 Daniel Veditz [:dveditz] 2005-04-20 00:09:24 PDT
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-7 (review requests must explain patch) 2005-04-22 15:43:45 PDT
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 [:bz] 2005-04-24 14:39:28 PDT
We have existing bugs on the fact that view-source:javascript: runs JS at all.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-04 13:06:17 PDT
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?
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-04 13:20:52 PDT
(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, jst@mozilla.com) 2005-05-04 13:25:25 PDT
FWIW, it seems like that code was added in bug 31818.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2005-05-04 13:48:29 PDT
Some of the comments there suggest that caps was then returning something more
like 'not a principal'.
Comment 10 Daniel Veditz [:dveditz] 2005-05-04 13:50:32 PDT
(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, jst@mozilla.com) 2005-05-04 16:07:56 PDT
(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 [:bz] 2005-05-04 19:35:18 PDT
> 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-7 (review requests must explain patch) 2005-05-05 12:00:15 PDT
cc:ing caillon, in case he has thoughts on comment 7.
Comment 14 Daniel Veditz [:dveditz] 2005-05-10 18:56:07 PDT
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 2005-05-10 19:07:11 PDT
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.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2005-05-10 19:23:19 PDT
Comment on attachment 183232 [details] [diff] [review]
v2 patch

sr=jst
Comment 17 Daniel Veditz [:dveditz] 2005-05-10 19:25:47 PDT
Comment on attachment 183232 [details] [diff] [review]
v2 patch

r=dveditz
Comment 18 Darin Fisher 2005-05-10 19:33:12 PDT
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.
Comment 19 Darin Fisher 2005-05-10 19:34:28 PDT
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 2005-05-11 04:06:15 PDT
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 2005-05-11 07:32:31 PDT
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) 2005-05-11 08:00:13 PDT
urg, branches use the netwerk/protocol/jar version (which is gone on trunk). Why
does nsJARChannel exist in modules/libjar?
Comment 23 Darin Fisher 2005-05-11 10:45:46 PDT
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] 2005-05-11 18:25:11 PDT
*** Bug 291618 has been marked as a duplicate of this bug. ***
Comment 25 Darin Fisher 2005-05-12 08:21:02 PDT
I checked in the v2 patch on the trunk.

Marking FIXED
Comment 26 Daniel Veditz [:dveditz] 2005-05-18 13:09:08 PDT
Clearing security flag from announced vulnerabilities fixed in Firefox
1.0.4/Mozilla 1.7.8
Comment 27 Bruce Robson 2005-05-20 08:12:28 PDT
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?

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