820094 - node of a documents from an XHR of a windowless sandbox throws

Status: RESOLVED FIXED

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

Description

[Gabor Krizsanits (INACTIVE)]

Starts here: https://bugzilla.mozilla.org/show_bug.cgi?id=817284#c7

The problem I'm struggling with is from bug 456271. So if I try to access the nodes of the responseXML, it throws. In my earlier test for these type of XHR objects I did not detect this, because I accessed the nodes from system code, not from the scope of the content sandbox. In jetpack the XHR of content-scripts will not work because of this I'm affraid :(

Bobby, can you tell me if we still need this restriction in nsDOMClassInfo? I don't see what is the security risk here...

Comment 1

[Boris Zbarsky [:bzbarsky]]

So in this case the doc never had a script handling object, right?  That means it's not associated with a particular window at all...  Olli, Peter, is it ok to just stamp a script handling object on the document in cases like this?  One problem is we don't really have one in sandboxes, I guess...

Comment 2

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky (:bz) from comment #1)
> So in this case the doc never had a script handling object, right?

Yes.

> One problem is we don't really have one in sandboxes, I guess...

And yes too... I should have found this issue a lot sooner, but my test avoided this unluckily.

Comment 3

[Gabor Krizsanits (INACTIVE)]

As smaug explained it to me on irc, the background story of this. Things like 
Bug 393762 and Bug 393761. I _think_ that these kind of problems should be taken care of by wrappers, but this is an area of the wrappers I know very little about how it works right now exactly. Bobby?

Comment 4

[Gabor Krizsanits (INACTIVE)]

I tested the situation from an addon. The difference is that as opposed to my earlier test now the sandbox has an nsEP as its principal, and hooked up the content window in it's proto chain. The nsEP ofc subsumes the content principal. Sadly things get extremely fishy very fast. 

    contentScript: "new " + function ContentScriptScope() {
      self.on("message", function (url) {
        let request = new XMLHttpRequest();
        request.open("GET", url, true);
        request.onload = function () {
          var v = request.responseXML.getElementsByTagName('root').item(0);
          self.postMessage(request.responseText);
        };
        request.send(null);
      });

So the responseXMK document will find the content window as it's script handling object I assume. So the document is builded fine, except content-script cannot access it. "Permission denied to access property 'getElementsBy
TagName'". I assume the onload function is called with the privileges of the content (because the content window on the proto chain) but the document is in the scope of the content-script (as well as the XHR object that received/created it).

Comment 5

[Boris Zbarsky [:bzbarsky]]

In that testcase the onload handler should run with the same permissions as the XHR open() call.

I'd love a testcase that actually reproduces this problem that I can install to see what's going on...

Comment 6

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky (:bz) from comment #5)
> In that testcase the onload handler should run with the same permissions as
> the XHR open() call.

This is what I would expect yes, so I don't understand the permission error, only guessing here. It can be some nsEP related security check somewhere I have not noticed yet, I want to dig into this as well just have some urgent paper work that distracts me right now :(

> I'd love a testcase that actually reproduces this problem that I can install
> to see what's going on...

I will deliver a mochi test as well but for now here is a scratchpad example that can be easily executed and debugged from scratchpad opened from some random page (in browser context ofc)

var cu = Components.utils;

var sb = cu.Sandbox([content.window, "http://www.w3schools.com"],
         {wantXHRConstructor: true, sandboxPrototype: content.window});

function createXHR()
{
  var xhr = new XMLHttpRequest();
  xhr.open("GET", "http://www.w3schools.com/XML/cd_catalog.xml", true);
  return xhr;
}

function checkResults(xhr)
{
  var root_node = xhr.responseXML.getElementsByTagName('CATALOG').item(0);
}

cu.evalInSandbox('var createXHR = ' + createXHR, sb);
cu.evalInSandbox('var checkResults = ' + checkResults, sb);

var async = cu.evalInSandbox('var testFinished = false; var async = createXHR(); ' +
                             'async.onload = function(){checkResults(async)}; async',
                             sb);
async.send(null);

Comment 7

[Bobby Holley (:bholley)]

This sounds like exactly the kind of "spurious security exception" I was predicting in bug 820170. If chrome is the first caller that creates a wrapper for the document, the XPCWN will end up stuck in chrome, and neither the content nor the content script will be able to access it.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> here is a scratchpad example

That throws from the responseXML getter, because it's failing the scope object stuff in PreCreate.  That doesn't match what comment 4 seems to be talking about....

Comment 9

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky (:bz) from comment #8)
> > here is a scratchpad example
> 
> That throws from the responseXML getter, because it's failing the scope
> object stuff in PreCreate.  That doesn't match what comment 4 seems to be
> talking about....

On m-c? I tested this on released version (FF17.01) and beta (FF18) and this is the exception: Permission denied to access property 'getElementsByTagName'. responseText works fine.

Comment 10

[Gabor Krizsanits (INACTIVE)]

Nevermind, I realized that you were probably just answering Bobby...

Comment 11

[Boris Zbarsky [:bzbarsky]]

> On m-c?

Yes.  And no, I was answering comment 6.  On m-c, the responseXML getter already throws.

Comment 12

[Gabor Krizsanits (INACTIVE)]

(In reply to Boris Zbarsky (:bz) from comment #11)
> > On m-c?
> 
> Yes.  And no, I was answering comment 6.  On m-c, the responseXML getter
> already throws.

Weird I wanted to check it on m-c but it was not building on windows, so I used inbound, but that should be quite similar I assume. So accidentally I forgot to set up a mozconfig so I assume I ended up with a release build, I'm not sure if all this matters or not but I did get the same exception by copy pasting my example into this inbound builds scratchpad. I got a bit lost in the debugging but I'll continue it tomorrow. The expression is from a CrossOriginAccessiblePropertiesOnly filter. So comment 7 can be it.

Comment 13

[Gabor Krizsanits (INACTIVE)]

I think/hope that bug 820170 will make this problem go away. I was a bit surprised thought how could in this setup chrome be the first caller. I would totally expect the sandbox to be the caller. So I debugged it and it turns out that the DocElementCreated event is the first time when the document gets wrapped, so the documents wrappers flatJSObject indeed ends up in the system compartment as Bobby predicted. I think a nice test can be made out of this example for that patch. Will deliver it tomorrow.

Comment 14

[Thomas Wisniewski]

Gabor, did this problem end up going away after bug 820170 was fixed, or is this bug still possibly happening?

Comment 15

[Gabor Krizsanits (INACTIVE)]

Yes, this has been fixed.

Comment 16

[Gabor Krizsanits (INACTIVE)]

Whoops, I accidentally made that last comment private.