Bug 1557393 Comment 7 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

> That comment is incorrect. 

OK, would be good to fix it.  And then _really_ carefully audit all the callsites, I guess?

> Isn't the original bug here caused by the Debugger failing to unwrap something it ought to be able to unwrap?

Yes.  But note that in the case in question in comment 0 the debuggee _can_ unwrap it.

> Perhaps the use of UnwrapOneCheckedStatic back in March was not correct.

Entirely possible; like I said in comment 0 its use was temporary pending figuring out what the actual behavior should be.

> and the Debugger API should be using mechanisms parallel to what real JS uses to make its decisions.

The mechanism JS uses for doing checked unwrap in full generality at this point is that it has a concept of "who is asking?" represented by the current Realm of the JSContext.  But more importantly, the real question needs to be "why are we doing the unwrap and what do we plan to do with the result?"  Arbitrary escape of unwrapped values is a likely security issue and needs to be considered very carefully.

In terms of what the behavior here _used_ to be it depends on what `referent` could be in `DebuggerObject::unwrap`.  Based on comments around that code it seems like `referent` can be a CCW; can I trust those comments?  If so, which compartment is it a CCW from/to?  For example, is `referent` always in the compartment of the `DebuggerObject` (and hence the compartment of the `JSContext` coming into `DebuggerObject::unwrap`?  Or can it be in a different compartment?  The answer to this determines what the old behavior, using `UnwrapOneChecked` was, since that used the security policy if the CCW in question when doing the unwrap.

I am here assuming that when we land in `DebuggerObject::unwrap` both `cx` and `object` are in the debugger compartment, not the debuggee compartment, right?

> There is no jit-test for D.O.p.unwrap-ing between different privilege levels 

The unwrap in comment 0 is not even between different privilege levels, but is on a very specific kind of cross-compartment proxy that does not exist in SpiderMonkey proper.
> That comment is incorrect. 

OK, would be good to fix it.  And then _really_ carefully audit all the callsites, I guess?

> Isn't the original bug here caused by the Debugger failing to unwrap something it ought to be able to unwrap?

Yes.  But note that in the case in question in comment 0 the debuggee _can_ unwrap it.

> Perhaps the use of UnwrapOneCheckedStatic back in March was not correct.

Entirely possible; like I said in comment 0 its use was temporary pending figuring out what the actual behavior should be.

> and the Debugger API should be using mechanisms parallel to what real JS uses to make its decisions.

The mechanism JS uses for doing checked unwrap in full generality at this point is that it has a concept of "who is asking?" represented by the current Realm of the JSContext.  But more importantly, the real question needs to be "why are we doing the unwrap and what do we plan to do with the result?"  Arbitrary escape of unwrapped values is a likely security issue and needs to be considered very carefully.

In terms of what the behavior here _used_ to be it depends on what `referent` could be in `DebuggerObject::unwrap`.  Based on comments around that code it seems like `referent` can be a CCW; can I trust those comments?  If so, which compartment is it a CCW from/to?  For example, is `referent` always in the compartment of the `DebuggerObject` (and hence the compartment of the `JSContext` coming into `DebuggerObject::unwrap`?  Or can it be in a different compartment?  The answer to this determines what the old behavior, using `UnwrapOneChecked`, was since that used the security policy if the CCW in question when doing the unwrap.

I am here assuming that when we land in `DebuggerObject::unwrap` both `cx` and `object` are in the debugger compartment, not the debuggee compartment, right?

> There is no jit-test for D.O.p.unwrap-ing between different privilege levels 

The unwrap in comment 0 is not even between different privilege levels, but is on a very specific kind of cross-compartment proxy that does not exist in SpiderMonkey proper.

Back to Bug 1557393 Comment 7