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

RESOLVED FIXED

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
a year ago

People

(Reporter: Gabor Krizsanits (INACTIVE), Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

5 years ago
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...
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...
(Reporter)

Comment 2

5 years ago
(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.
(Reporter)

Comment 3

5 years ago
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?
(Reporter)

Comment 4

5 years ago
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).
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...
(Reporter)

Comment 6

5 years ago
(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);
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.
> 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....
(Reporter)

Comment 9

5 years ago
(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.
(Reporter)

Comment 10

5 years ago
Nevermind, I realized that you were probably just answering Bobby...
> On m-c?

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

Comment 12

5 years ago
(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.
(Reporter)

Comment 13

5 years ago
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.
Depends on: 820170

Comment 14

a year ago
Gabor, did this problem end up going away after bug 820170 was fixed, or is this bug still possibly happening?
Flags: needinfo?(gkrizsanits)
(Reporter)

Comment 15

a year ago
Yes, this has been fixed.
Status: NEW → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(gkrizsanits)
Resolution: --- → FIXED
(Reporter)

Comment 16

a year ago
Whoops, I accidentally made that last comment private.
Comment 15 is private: false
You need to log in before you can comment on or make changes to this bug.