296830 - same origin violation: child frames calling rewritten top.focus() [sa15549]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Veditz [:dveditz]]

Secunia Research has discovered a vulnerability in Mozilla Suite, Firefox and
Netscape, which can be exploited by malicious people to conduct cross-site
scripting attacks.

The problem is that the "frames", "parent", "self" and "top" objects are not
properly protected from being modified by another site via JavaScript. This can
be exploited to execute arbitrary HTML and script code in a user's browser
session in context of an arbitrary site, which calls a method in one of the
modified objects.

The vulnerability has been confirmed in Mozilla Suite 1.7.8, Firefox 1.0.4 and
Netscape 8.0.1. Other versions may also be affected.

-- PoC (Proof of Concept) --
1. Go to SA15549.zip/poc.html and click link.
2. poc_b.html loads amazon.com in an invisible iframe and modifies "top.focus()"
which runs on load.
NOTE: Exploitation is site specific (choose a suitable object to manipulate).
3. An alert box shows location and cookie of Amazon. The address bar is also
spoofed.

Discovered by: Andreas Sandblad, Secunia Research

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: testcase steals amazon cookies

Comment 2

[Daniel Veditz [:dveditz]]

You don't need to overwrite victim.top.focus, you can simply replace
window.focus on the attacking page and the child frame will call it.

The address bar isn't exactly spoofed, the frame is calling the attack function
which does a document.write(). The URL bar is correctly updated to show the
document that is responsible for the writing--in this case the frame, which is
why it also has access to the frame's cookies. The problem is why we think
amazon is the origin for that focus call, and if we do why we let it call a
property written by someone else.

Mozilla 1.0 (Netscape 7.02) is not vulnerable, but Mozilla 1.4 (Netscape 7.1)
is. Probably too big a window to provide much clue about where this regressed.

Comment 3

[Daniel Veditz [:dveditz]]

About a year ago we fixed bug 86028 for 1.7 which affects access to these events

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/libpref/src/init/all.js#265

I don't think that's related, wrong timeframe and the frame isn't trying to
change the event handler, it's just being called. document.write() appears to be
the culprit, setting the wrong owner for the wyciwyg document. Inside the
focus() routine the environment appears to be the outerframe as I'd expect.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Is 

  270 pref("capability.policy.default.Window.focus.get", "allAccess"); 

in all.js part of the problem?  That's needed so sites can call focus() on a
window they opened which may be in a different domain (to raise it, eg).

All that said, if we really want to track down the regression we might be able
to -- I have 370-some nightlies from the 13-month period between 1.0 and 1.4,
which should give decent coverage. Unfortunately, I can't really test till end
of June; fortunately if someone does they are also at http://archive.mozilla.org
(and doing a binary search should involve downloading only about 10 builds).

Comment 5

[Johnny Stenback (:jst)]

No capabilities changes will help here at all. A evil page can simply set its
own focus method and load a victim page in a frame, once the frame calls
top.focus() we're in the same situation.

Seems like IE does a security check when *accessing* window.focus to see if the
caller is from the same origin as the origin of the *value* of the property
(window.focus). Fun...

Comment 6

[Johnny Stenback (:jst)]

Attachment: Use the actual calling principal when replacing a document's principal in document.open().

This was way easier than I first thought, at least to plug the hole. With this
change we still work differently than IE and Opera does, but there's no longer
a security problem. IE and Opera throw security exceptions when using this
testcase, we don't. We simply do what we intended to do in this case all along,
but do it correctly. The problem was that when calling document.open() (or
.write()) on a document that's already loaded, we give the new document the
principals of the caller, but we got the caller wrong, we used the document in
the calling context, when we should use the actual principal of the caller.
That way no data theft like this one is possible any more.

Comment 7

[Johnny Stenback (:jst)]

For the record, I filed bug 298064 on a related problem.

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 186664 [details] [diff] [review]
Use the actual calling principal when replacing a document's principal in document.open().

Yup, straight logic bug fix -- old bug (vidur didn't read the libmocha code
hard enough! ;-).  Any others like it?

/be

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 186664 [details] [diff] [review]
Use the actual calling principal when replacing a document's principal in document.open().

r=dveditz

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 186664 [details] [diff] [review]
Use the actual calling principal when replacing a document's principal in document.open().

a=dveditz

Comment 11

[Johnny Stenback (:jst)]

Attachment: Same change (different place) for the branches.

Comment 12

[Johnny Stenback (:jst)]

Fixed on trunk and branches.

Comment 13

[Jay Patel [:jay]]

v.fixed on aviary with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.9)
Gecko/20050622 Firefox/1.0.5

Comment 14

[Daniel Veditz [:dveditz]]

Adding distributors

Comment 15

[Daniel Veditz [:dveditz]]

FF1.0.5 advisories published

Comment 16

[Boris Zbarsky [:bzbarsky]]

bclary, you set in-testsuite+, but there's no indication which test suite that is... Which one is it?

Comment 17

[Bob Clary [:bc] (inactive)]

(In reply to comment #16)
> bclary, you set in-testsuite+, but there's no indication which test suite that
> is... Which one is it?
> 

It is in an internal only set of tests that were to be the basis for an internal security regression test suite. The future of that set of tests and test suite is uncertain.

Comment 18

[Boris Zbarsky [:bzbarsky]]

I really think we should put all tests into the public test frameworks, especially when the bug is opened up.  This test should probably be added to mochitest?

Comment 19

[Boris Zbarsky [:bzbarsky]]

For what it's worth, the test doesn't really test this bug on trunk anymore, because window.focus on a wrapper is the native focus function, as far as I can tell.