403167 - XSS by using XMLDocument.load() and event handler

Status: RESOLVED FIXED

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

Description

[moz_bug_r_a4]

The fix in bug 393762 stops the testcases in bug 393761 (both XSS and privilege
escalation).  But, with that fix, it's still possible to perform XSS attacks. 


1. Create an XML document (D) whose script handling object is a subframe's
   global object, and attach an onload event handler to D.
2. Load a target site in the subframe.
3. Call D.load().

The onload event handler is called with the target site's principal.

Comment 1

[moz_bug_r_a4]

Attachment: testcase 1 - DOMImplementation.createDocument()

This tries to get cookies for www.mozilla.com.
This works on trunk.

Comment 2

[moz_bug_r_a4]

Attachment: testcase 2 - DOMParser.parseFromString()

This tries to get cookies for www.mozilla.com.
This works on trunk (and fx2.0.0.x and fx1.5.0.x).

Comment 3

[moz_bug_r_a4]

Attachment: testcase 3 - XMLHttpRequest.responseXML

This tries to get cookies for www.mozilla.com.
This works on trunk.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Hmm, are there any testcases where information leaks from iframe to
main page? I mean something where the data from iframe is really set to
some variables in the main page, not just alerts using data from
iframe.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #4)
> not just alerts using data from iframe. 
nevermind, idiot me. Jst clarified this in Bug 403168

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Attachment: WIP

Fixes testcases. I'm not fully happy with the patch, but I think we have similar 
problems anyway: if the scriptglobal, to which the document was connected, goes 
away, javascript handlers in data documents can't be executed.
The scriptglobal could be originally got from stack, not from the document which
creates the document, but that would just move the problem to different kind of
testcases, not remove it.
But I need to make a testcase still, to ensure that this XSS is not possible
with normal documents.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 288217 [details] [diff] [review]
WIP

Jst, can you think of anything better?
We don't have similar problem with normal documents, only with data documents. (At least couldn't write testcase to show that kind of bug.)

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

The reason why the XSS doesn't work with non-data-documents is that their script global object gets cleared. Because GlobalWindow doesn't know about
data-documents, it can't clear anything in them.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Hmm, perhaps taking context (which has ok principal) from stack in case data 
document doesn't have scripthandlercontext would be good to prevent some
possible regressions. 
Jst, any comments to that? It is not perfect solution, but would keep
data documents usable even if the context for which they were created
was lost.

Comment 10

[Daniel Veditz [:dveditz]]

XSS bugs are sg:high, sg:critical only if you can inject into a chrome frame (which allows arbitrary code execution)

Olli's not expressing a lot of confidence in these patches, and we've got two similar bugs so there may be more in this area. Getting it right for 2.0.0.11 would be better than rushing these patches into 2.0.0.10

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

The patch should fix things, the problem is that it and bug 393762 disables
some features - using event handlers with documents which are originally
created in a different context.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Ok, I've done some testing in 1.8, and there event handling doesn't work
(in normal cases) either with data or normal documents when their |window| has 
been teared down.
###!!! ASSERTION: XPConnect is being called on a scope without a 'Components' property! is thrown there.
That is good, we can expect and hope the fix here doesn't regress anything, and
documents shouldn't IMO be used when their context isn't alive anymore.

Note to myself, be careful when setting the event handling context.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 288217 [details] [diff] [review]
WIP

Nothing better comes to mind. r+sr=jst

Comment 14

[moz_bug_r_a4]

Even when a different context is not used, onload event handlers can't be
executed on documents which are originally created by DOMParser and
XMLHttpRequest.  Is this behavior intended?

Comment 15

[moz_bug_r_a4]

Attachment: testcase 4

fx-3.0b2pre-2007-11-15-05:
DOMParser : onload event handler was called
XMLHttpRequest : onload event handler was called
createDocument : onload event handler was called

fx-3.0b2pre-2007-11-16-05:
DOMParser : ---
XMLHttpRequest : ---
createDocument : onload event handler was called

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

No, that is a regression. I'm fixing it.
Thank you for testing!

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Attachment: ensure that inner window is used in data documents

Comment 18

[moz_bug_r_a4]

Hmm, after applying the second patch, the first patch can be circumvented by
using DOMParser or XMLHttpRequest on the subframe's context after loading a
target site in the subframe.

Comment 19

[moz_bug_r_a4]

Attachment: testcase 5 - DOMParser.parseFromString()

This works with "ensure that inner window is used in data documents" patch
applied.

Comment 20

[moz_bug_r_a4]

Attachment: testcase 6 - XMLHttpRequest.responseXML

This works with "ensure that inner window is used in data documents" patch
applied.

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

When the latest XHR-XSS patch is applied, testcase 6 doesn't work.
Now fixing testcase 5...

Comment 22

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 289147 [details] [diff] [review]
ensure that inner window is used in data documents

The patch to fix the callers are in the XHR-XSS bug.

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

The "WIP" patch for 1.8 is in Bug 393762.

Comment 24

[moz_bug_r_a4]

Now, testcase 5 and 6 should work on current 1.8 branch build.  On Windows XP,
those work as expected, but on Linux, testcase 5 does not work due to the fact
that subframe's document is still about:blank when calling
DOMParser.parseFromString().  I'll attach a new version of testcase 5, which
works both on Windows XP and Linux.

Comment 25

[moz_bug_r_a4]

Attachment: testcase 7 (testcase 5 for 1.8 branch on Linux) - DOMParser.parseFromString()

This works on:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12pre) Gecko/20080125
BonEcho/2.0.0.12pre
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.12pre) Gecko/20080125
BonEcho/2.0.0.12pre

Comment 26

[Samuel Sidler (old account; do not CC)]

Pushing this out into the next release.

Comment 27

[Al Billings [:abillings - ex-MoCo]]

I checked this in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13) Gecko/2008031114 Firefox/2.0.0.13.

For test cases 2, 3, and 4, the behavior is exactly the same between Firefox 2.0.0.12 and 2.0.0.13. There is no change.

For test cases 5, 6, and 7, there is no alert with cookie information in Firefox 2.0.0.13, which I do see in Firefox 2.0.0.12.

It isn't clear to me whether the behavior being the same for test cases 2, 3, and 4 means that this is only partially fixed. All I see in both versions in those cases is either the iframe with the page being displayed (and nothing else) or all three events being called.

Comment 28

[moz_bug_r_a4]

The patch for 1.8 branch in bug 393762 comment #25 has included the first patch
in this bug, thus testcase 1, 2 and 3 has been fixed in fx2.0.0.12.  testcase 4
demonstrated a regression, which did not happen in fx2.0.0.12.

Comment 29

[Al Billings [:abillings - ex-MoCo]]

I see the same behavior with testcase 4 for both 2.0.0.12 and 2.0.0.13.

Firefox 2.0.0.12 shows:

DOMParser : onload event handler was called
XMLHttpRequest : onload event handler was called
createDocument : onload event handler was called

Firefox 2.0.0.13 shows:

DOMParser : onload event handler was called
XMLHttpRequest : onload event handler was called
createDocument : onload event handler was called

Comment 30

[moz_bug_r_a4]

That is expected behavior.  The regression happened only on trunk when the
first patch in this bug landed.

Comment 31

[Al Billings [:abillings - ex-MoCo]]

Thanks!

I've marked this as verified for 1.8.1.13.

Comment 32

[Eugene Minaev]

how can i clone scriptGlobalObject in javascript or where i can read about scriptGlobalObject

Comment 33

[Johnny Stenback (:jst)]

What?

Comment 34

[Eugene Minaev]

how can i create an XML document whose script handling object is a subframe's
global object ?

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

frameElement.contentDocument.implementation.createDocument(...)