946752 - dbgObject.unsafeDereference() returns objects with no xray wrappers for a few DOM objects

Status: RESOLVED WORKSFORME

(DevTools :: Debugger, defect)

Description

[Mihai Sucan [:msucan]]

If I do dbgObject.unsafeDereference() from the debugger ObjectActor I don't get xray wrappers as expected for a few DOM objects. According to bug 945920 I should get xray wrappers for any object that is not a "pure JS object".

Examples:

- StyleSheetList - document.styleSheets
- DOMStringList - document.styleSheetSets
- CSSMediaRule, CSSStyleRule and CSSImportRule - just add a style sheet into your page with normal style rules, selectors, @media rules and @import rules.
- and there's more IIANM.

I do get xray wrappers for document, window, DOM nodes, elements and more. However, it is unexpected to see DOM stuff with no xray wrapping.

Why is this the case? I tried wantXrays: true in the addon-sdk loader.js Sandbox [1] and it seems to make no difference.



[1] addon-sdk/source/lib/toolkit/loader.js

Comment 1

[Mihai Sucan [:msucan]]

Attachment: bug-946752-testcase.diff

This is the test case patch.

- apply the patch, update your Firefox build.
- in the webconsole evaluate document or document.body. Notice isXrayWrapper = true.
- now eval document.styleSheets. Notice isXrayWrapper = false.
- you can try other objects. Play with CSS stylesheets, expand cssRules, see what you get for style/media/import rules.

Maybe I am missing something. Why would they differ?

Thank you!

Comment 2

[Bobby Holley (:bholley)]

At least in the old days, I'm pretty sure that things like CSS rules were per-scope XPCWNs, that we would just regenerate across compartment boundaries (which would explain the lack of Xrays). But I'm not sure what they look like in the WebIDL world. Boris?

Comment 3

[Boris Zbarsky [:bzbarsky]]

CSS rules are one of the few things not on WebIDL yet.  So they're still doing the per-scope XPCWN thing.

In fact, looking at the current status at https://etherpad.mozilla.org/classinfo it looks like everything listed in comment 0 has simply not been converted to WebIDL yet.

Comment 4

[Mihai Sucan [:msucan]]

Thank you for the info, bz.

Before WebIDL we had no xray wrapping for objects? What is per-scope XPCWN? Is there a way to safely access properties and methods from such objects?

Comment 5

[Gabor Krizsanits (INACTIVE)]

(In reply to Mihai Sucan [:msucan] from comment #4)
> Before WebIDL we had no xray wrapping for objects? What is per-scope XPCWN?
> Is there a way to safely access properties and methods from such objects?

Oh yeah, I forgot about this case, sorry about that. Long story short, you're safe.

A simplified version of the story is that for each native object there is a reflector object in js that forwards the calls from js to c++, and also holds the js based modifications (we called those expandos). Cross compartment we can either look at the original reflector with xray vision on, or generate another new reflector in the other compartment instead. In the end no matter which happens you don't have to worry about the modifications content made on these natives, they won't affect you.

Comment 6

[Gabor Krizsanits (INACTIVE)]

So what Mihai is trying to read properties safely without having to worry about executing content script accidentally.

So I'm trying to sum it up...

For natives I don't know any API that can tell apart a waived xray from a per-scope XPCWN. As in we don't have an isWaiving function, so it can be difficult to tell if we are safe in the xray sense or not. What I can suggest is to call Cu.unwaiveXrays on it first, which strips off any waivers if there are any. That will guarantee the safe access of native properties. Note, that xray will let you access only to DOM properties by spec (things that have a native implementation), no custom content defined ones (those that are added from content JS).

Now for pure JS objects you need to do a little bit more dance. Calling Cu.unwaiveXrays on them is harmless, actually probably a good thing for you, since it could get rid of waiver flag that it would transit to natives. But that does not make it safe to access any properties on it... What you can do is either structured clone it to your scope (which might fail...) and read the properties from the cloned version. Or you can sniff around with getPropertyDescriptor and get the property only if it's defined as value, and there are no gettter function defined. Even then you need to be careful about proxy object that might interrupt even your getPropertyDescriptor call. To check for those there is recently landed Cu.isProxy() function.

Bobby, can you correct/confirm my statements here when you have some time for it?

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Before WebIDL we had no xray wrapping for objects? 

We basically had it for some objects but not others.  For the ones where we had no Xray wrapping, we just created a completely separate JS object for chrome to look at, like comment 5 says.

> What is per-scope XPCWN?

A setup where every Window looking at the given C++ object gets its own separate JS object.

Comment 8

[Mihai Sucan [:msucan]]

Thank you Gabor and Boris!

For pure JS objects that are not safe to access I'm using the debugger APIs for reading their properties. I can safely avoid triggering getters and the likes.

(In reply to Boris Zbarsky [:bz] from comment #7)
> > Before WebIDL we had no xray wrapping for objects? 
> 
> We basically had it for some objects but not others.  For the ones where we
> had no Xray wrapping, we just created a completely separate JS object for
> chrome to look at, like comment 5 says.
> 
> > What is per-scope XPCWN?
> 
> A setup where every Window looking at the given C++ object gets its own
> separate JS object.

Thanks! Is there any API I can use to tell the difference from 'pure JS objects' that are not safe to access and these JS objects which have per-scope XPCWN?

I need this so I can determine when to use the debugger API to read properties, versus when the JS object is safe and I can just read the properties directly. In the majority of cases I care about the native properties and methods.

Comment 9

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #5)
> A simplified version of the story is that for each native object there is a
> reflector object in js that forwards the calls from js to c++, and also
> holds the js based modifications (we called those expandos).

In the per-scope XPCWN case, we've always disallowed expandos, which is why the trick of re-generating a new reflector in each compartment is invisible to script.

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #6)
> So what Mihai is trying to read properties safely without having to worry
> about executing content script accidentally.

Yeah, for those objects it's totally fine. You can't even stick expandos on them, so content wouldn't have been able to do anything nasty even if the objects were shared.

> For natives I don't know any API that can tell apart a waived xray from a
> per-scope XPCWN.

Cu.isXrayWrapper?


(In reply to Mihai Sucan [:msucan] from comment #8)
> Thanks! Is there any API I can use to tell the difference from 'pure JS
> objects' that are not safe to access and these JS objects which have
> per-scope XPCWN?
> 
> I need this so I can determine when to use the debugger API to read
> properties, versus when the JS object is safe and I can just read the
> properties directly. In the majority of cases I care about the native
> properties and methods.

I think you basically want something like the following:

function isSafeFromUntrustedModifications(obj) {
  if (Cu.getGlobalForObject(obj) == window)
    return true; // obj is not a cross-compartment wrapper.
  if (obj != Cu.unwaiveXrays(obj))
    return false; // obj is Xrayable, but waived.
  return Cu.isXrayWrapper(obj);
}

Comment 10

[Gabor Krizsanits (INACTIVE)]

(In reply to Bobby Holley (:bholley) from comment #9)
> Cu.isXrayWrapper?

I find it a bit counterintuitive that this returns true for waived xrays.

Comment 11

[Bobby Holley (:bholley)]

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #10)
> (In reply to Bobby Holley (:bholley) from comment #9)
> > Cu.isXrayWrapper?
> 
> I find it a bit counterintuitive that this returns true for waived xrays.

It most definitely does not, AFAIK, and according to the basic testing I just did. Do you have a situation where that happens? If so, we should fix that.

Comment 12

[Mihai Sucan [:msucan]]

Thank you all for the answers. The Cu.getGlobalForObject() check did the trick. Now I updated my local patch to use that check and it is working as desired.

(marking the "bug" as resolved)