54237 - Event doesn't fire from XUL file but does when loaded from HTML file

Status: VERIFIED FIXED

(Core :: Security: CAPS, defect, P3)

Description

[Peter Van der Beken]

If you put these four files somewhere in your chrome folder, and load the 
test.html file, it works ok. If you load the test.xul file, it doesn't. Use 
chrome:// URLs to load the files. You'll have to edit test.html and test.xul to 
point to the right location for test.js.

Expected result: alert box that says "Test"
Actual result: no alert box when using .xul file

The event is fired (from nsXMLDocument::EndLoad), but is stopped by 
nsScriptSecurityManager in CheckFunctionAccess (returning 
NS_ERROR_DOM_SECURITY_ERR). The difference between the two is when 
using a html file, the test (if (isSameOrigin)) at line 640 of mozilla/
caps/src/nsScriptSecurityManager.cpp succeeds, and for a xul file it fails.

This is currently blocking my work on the chrome for Transformiix (XSLT 
processor).

Comment 1

[Peter Van der Beken]

Attachment: test.html - works ok

Comment 2

[Peter Van der Beken]

Attachment: test.xul - doesn't work ok

Comment 3

[Peter Van der Beken]

Attachment: test.js

Comment 4

[Peter Van der Beken]

Attachment: test.xml

Comment 5

[Mitchell Stoltz (not reading bugmail)]

dup of 53493. I have a patch, waiting on approval. Peter, if you need a quick
fix, apply my patch from 53493. 

*** This bug has been marked as a duplicate of 53493 ***

Comment 6

[Scott Brodmerkle]

Verified dupe.

Comment 7

[Peter Van der Beken]

Hmm, actually, this is not a dupe. Bug 53493 has to do with html files not 
working from inside chrome. I have no problem with the html file, it's when 
using the xul file that it doesn't work. Reopening.

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Nonetheless, this will be fixed by 53493, I think. 

Comment 9

[Peter Van der Beken]

Ok, because you're so persistent, I'll try out the patch to 53493.

Comment 10

[Peter Van der Beken]

I tried the patch to 53493 and as I expected, it doesn't fix this bug at all. I 

don't think they are related.

Comment 11

[Marcus G. Daniels]

For me, `load' only works when the URL is a simple file name or of 
the file:// syntax, and the load is contained in a HTML file.  I can't
get JavaScript loaded from XUL files to send notifications.  Tracing through
nsXMLDocument::Load, it looks like it passes the security checks..

Comment 12

[selmer (gone)]

rtm-, I don't see that any existing xul is impacted by this bug.  Getting a fix
into the trunk for Transformiix is a good thing though.

Comment 13

[John Unruh]

Mass changing QA to ckritzer.

Comment 14

[Joe Hewitt (gone)]

I was overjoyed to encounter this bug today.  Here's what I found is and is not 
possible:

I CAN load a file from a file:// html document.
I CAN NOT load a file from a chrome:// html document.
I CAN load a file from a file:// xul document.
I CAN NOT load a file from a chrome:// xul document.

So, in other words, regardless of the document type, xml loading through script 
fails when your app document is loaded through chrome://

Comment 15

[Joe Hewitt (gone)]

I've traced the problem down a little further.  As Peter stated in the bug 
report, the Equals at line 640 in nsScriptSecurityManager is failing when the 
source document comes from chrome.  This is because the two principals being 
compared in this case are a SystemPrincipal and an AggregatePrincipal.  The 
SystemPrincipal comes from the function that was declared in the chrome 
document.  The AggregatePrincipal comes from the document object that is the 
target of the "load" event, and it contains a CodebasePrincipal that points to 
the xml document you have loaded. 

It looks to me that the security manager is rejecting this event handler 
because it assumes the CodebasePrincipal is some evil web document that is 
trying to intercept events from the SystemPrincipal.  In this case, this is 
obviously not so.  What needs to happen is that when an xml document is created 
for data purposes only, it's js object should have the same principal as the 
context where it was created.  Since this document will never be presented, and 
therefore, never be able to do evil things to the user, it should not need to 
have an AggregatePrincipal all the time.

CC'ing the XML DOM guys since this is getting into their domain.

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Joe,
  Could you post a testcase for the situation you described (the one that doesn't 
work)?

Comment 17

[Peter Van der Beken]

Mitch, i think you can use the existing testcases to try it out. Put them
somewhere in the chrome directory and try to load test.xul with a chrome URL.
Now that Joe seems to have found what the problem is, any chance we could get a
fix? If you don't have time for this, could you see if Joe's solution is correct
and show us where to implement it?

Comment 18

[Mitchell Stoltz (not reading bugmail)]

Mass adding mozilla0.9 keyword (mass changing milestone doesn't seem to work).

Comment 19

[Mitchell Stoltz (not reading bugmail)]

Setting milestone to Moz0.9.

Comment 20

[Mitchell Stoltz (not reading bugmail)]

Attachment: Here's a fix for chrome accessing chrome and chrome accessing non-chrome.

Comment 21

[Mitchell Stoltz (not reading bugmail)]

Two parts to the above patch. The nsXMLDocument part gets the new document's
principal from the channel that loaded it if the channel provided a principal.
This is the same behavior as a normal document load in a browser window. This
way, if the channel assigns the system principal (as a chrome load will do), the
generated XML document will be given the system principal.

The other part is to add a check for system principal to
nsScriptSecurityManager::CheckFunctionAccess. This is the security check
function called when avent handlers are about to be run. THe system principal
should be able to capture events from any domain.

Comment 22

[John Bandhauer]

sr=jband

Comment 23

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I can't say security is my strong field, but it looks ok to me, r=heikki.

Comment 24

[Mitchell Stoltz (not reading bugmail)]

Fix checked in.

Comment 25

[Peter Van der Beken]

It works now, thanks Mitchell. Marking verified.