Closed Bug 945920 Opened 11 years ago Closed 11 years ago

dbgObject.unsafeDereference() returns raw objects with no xray wrappers

Categories

(DevTools :: Debugger, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: msucan, Unassigned)

Details

Attachments

(1 file)

Given dbgObject, a Debugger.Object, let raw = dbgObject.unsafeDereference(). At this point one can do raw.foobar() - you can execute content scripts. |raw| is not xray wrapped.

jimb tells us unsafeDereference() should return xray wrapped objects, yet, testing shows otherwise.

I would need this stuff to work properly for bug 843004. Any ideas why this is not working as expected?
Here is a patch with a test case.

Apply this patch, build Firefox and load any page.

Execute the following in webconsole:

  foo = { lolz: function() { alert('yay'); } }

see the alert() twice.
Gabor, would it be easy for you to determine why |raw| is not a wrapper? and why the XPCNativeWrapper() constructor wouldn't work? See the attached test case.

I found the addon-sdk/source/lib/toolkit/loader.js script which is used for loading the devtools files. This script creates sandboxes with wantXrays: false. I changed that to true, and there's no change. Scripts are loaded into the sandboxes using loadSubScript().
Flags: needinfo?(gkrizsanits)
(In reply to Mihai Sucan [:msucan] from comment #2)
> Gabor, would it be easy for you to determine why |raw| is not a wrapper? and
> why the XPCNativeWrapper() constructor wouldn't work? See the attached test
> case.

So if I get it right unsafeDereference returns the object being debugged, without the debugger wrapper around it, is that right?

Xrays work (currently) only for native object, by that I mean things like DOM node, or anything else that has a C++ implementation behind we can xray to. Your object is just a pure JS object so there is no xray for it.

Also, if the sandbox you are executing your code in, has the wantXrays: false flags, it will waive xrays even for natives by default. You can call Cu.unwaiveXrays on them, to get back the xray. Although I wonder why is that flag false... 

Does that answers your question?

> 
> I found the addon-sdk/source/lib/toolkit/loader.js script which is used for
> loading the devtools files. This script creates sandboxes with wantXrays:
> false. I changed that to true, and there's no change. Scripts are loaded
> into the sandboxes using loadSubScript().

I really would like to know why do we use wantXrays: false for the loader. And since when? I flag Irakli for need info to answer this question. In general I'm not very happy about this idea, but I guess there is a reasonable explanation for it.
Flags: needinfo?(gkrizsanits) → needinfo?(rFobic)
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3)
> (In reply to Mihai Sucan [:msucan] from comment #2)
> > Gabor, would it be easy for you to determine why |raw| is not a wrapper? and
> > why the XPCNativeWrapper() constructor wouldn't work? See the attached test
> > case.
> 
> So if I get it right unsafeDereference returns the object being debugged,
> without the debugger wrapper around it, is that right?

Right.


> Xrays work (currently) only for native object, by that I mean things like
> DOM node, or anything else that has a C++ implementation behind we can xray
> to. Your object is just a pure JS object so there is no xray for it.
> 
> Also, if the sandbox you are executing your code in, has the wantXrays:
> false flags, it will waive xrays even for natives by default. You can call
> Cu.unwaiveXrays on them, to get back the xray. Although I wonder why is that
> flag false... 
> 
> Does that answers your question?

This has certainly improved my understanding of what I should expect.

> > I found the addon-sdk/source/lib/toolkit/loader.js script which is used for
> > loading the devtools files. This script creates sandboxes with wantXrays:
> > false. I changed that to true, and there's no change. Scripts are loaded
> > into the sandboxes using loadSubScript().
> 
> I really would like to know why do we use wantXrays: false for the loader.
> And since when? I flag Irakli for need info to answer this question. In
> general I'm not very happy about this idea, but I guess there is a
> reasonable explanation for it.

Even if wantXrays is false I just tested with a DOM element, and I do get those objects xray wrapped. This is interesting.

Now why is this difference wrt. pure JS objects versus native objects? Doesn't this defeat the guarantee of always having content objects wrapped, safe from executing content scripts by mistake? Should one always check if Cu.isXrayWrapper(obj)? If yes, this is a big 'gotcha'.

Thank you very much for your information/help with this.
(In reply to Mihai Sucan [:msucan] from comment #4)
> 
> Even if wantXrays is false I just tested with a DOM element, and I do get
> those objects xray wrapped. This is interesting.

Uhh... I just realized I was not precise. wantXrays flag has only effect on objects from a
same origin compartment, for the cross origin case this flag is ignored. I assume in your case
the element is from content and the sandbox has system principal, so we use xray regardless of
the flag.

> 
> Now why is this difference wrt. pure JS objects versus native objects?

Xray is about forgetting all the JS mutations that was done by content on the holder object (the JS object that
connects JS to the native implementation of the element), and calls right into the native functions. Simply put since we know where is let's say appendElement implemented in C++ we can just call it regardless of how content might have overwritten it at JS level. Now for pure JS object there is not really any native functions we can fall back to. In your case lolz: function(){...} property once overwritten it's not possible to get back the original value of it, or even define what the 'original' means...

You can however implement COM object in JS and then you can fake natives, but it's not a lot of fun to do so.

Finally, Bobby is working on some kind of xrays for JSObjects, because for a few things they make sense. Like Object.prorotype stuff. But I doubt it will help you here.

> Doesn't this defeat the guarantee of always having content objects wrapped,
> safe from executing content scripts by mistake? Should one always check if
> Cu.isXrayWrapper(obj)? If yes, this is a big 'gotcha'.
> 
> Thank you very much for your information/help with this.

Unfortunately there is no guarantee that content code will not be executed if you poke a pure JS object
from content code. You can try to be very careful like checking property descriptor before getting a property
if there is a getter function defined for it... but then you still need to watch out for proxy object that might
interrupt your getPropertyDescriptor call. For detecting that I've just landed a Cu.isProxy() that checks if an JSObject is a scripted proxy or not... I wish I had a silver bullet here, but this is one of the hardest problem for our security layer to deal with properly. You can also try to structure clone the object to your scope and then access it safely, we have an API for that, but it comes with an overhead ofc, and I'm not sure that is what you need.

I wonder what is your real goal here with the unsafeDereference exactly, I might be able to help you find a solution for it if I know more about the problem you are facing with.
Gabor, thanks for the details. I'm working on bug 843004 where I need to read properties from content objects. For DOM objects I wanted to make sure I get xray wrappers before I read elem.nodeName, or elem.className, etc. I would've liked something similar for pure objects - eg. given a RegExp I wanted to be sure that I execute toString() or whatever I need.  However, for pure JS objects as an alternative I can use native functions. For example, RegExp.prototyp.toString.call(contentJSObject).

Nonetheless, I'm sure now I can manage with what I have. Marking the bug as invalid - it seems this is expected behavior for pure JS objects.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → INVALID
I clear the need info since I realized that not using xrays only happens for natives from the system scope, so it's probably OK.

Ok, let's continue it on bug 843004. There are a few things that hard to get right in this area, so feel free to r?/f? flag me or Bobby anytime if you have any concerns.
Flags: needinfo?(rFobic)
Thank you!
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: