evaluating "new Date()" using evalInSandbox fails

VERIFIED WORKSFORME

Status

()

Core
Security
P2
normal
VERIFIED WORKSFORME
10 years ago
8 years ago

People

(Reporter: Dolske, Assigned: mrbkap)

Tracking

Trunk
Points:
---
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
Spunoff from bug 421593.

Evaling code like "new Date()" or "new XMLHttpRequest();" in a sandbox can unexpectedly throw a NS_ERROR_XPC_NOT_ENOUGH_ARGS error.

The following is sufficient to recreate the problem (when run from a chrome context, where |domWin| is a DOM window for some untrusted page):

var sandbox = new Components.utils.Sandbox(domWin);
sandbox.__proto__ = domWin.wrappedJSObject;
var result =  Components.utils.evalInSandbox("new Date()", sandbox);

The problem seems to be a result of setting __proto__ to the wrappedJSObject. If the above code uses |sandbox.__proto__ = domWin;| it works fine. However, that isn't an acceptable workaround, because then evaluating code like "foo = 1" sets foo on the XPCNativeWrapper, where the page content can't see it.
Flags: blocking1.9?

Comment 1

10 years ago
While I agree that this needs to be fixed, it does not need to block 421593 since I have an implementation for the commandLine that does not use evalInSandbox.
(Reporter)

Comment 2

10 years ago
Created attachment 313201 [details] [diff] [review]
Testcase

I'm not sure what the easiest way to tests this is, since you need both (1) chrome context and (2) a DOM window. So, this is rather hacky, but since I know password manager code I just stuck this in there. :-) The WebProgressListener where I stuck this gets called during page load.
(Reporter)

Updated

10 years ago
Assignee: dveditz → mrbkap
What else does this break?  Since firebug is working around this issue, can we do this in 1.9.0.x?

Comment 4

10 years ago
It seems like this would break greasemonkey at least, depending on the userscript.  I'd tend to agree that this should block.  Not sure why no one else has complained about it already.
+'ing based on Comment #4.  

Blake, you get to pick who fixes this because there's no way you have time.  Who's it going to be?
Flags: blocking1.9? → blocking1.9+
Fixing priority to P2.
Priority: -- → P2

Comment 7

10 years ago
changed title so I can find it
Summary: evaluating "new Date()" in a sandbox fails → evaluating "new Date()" using evalInSandbox fails

Comment 8

10 years ago
I added dolske's patch:

var sandbox = new Components.utils.Sandbox(targetWindow);
sandbox.__proto__ = targetWindow.wrappedJSObject;
try {
  result = Components.utils.evalInSandbox(expr, sandbox)
} catch (e) {
  result = "XXX threw: " + e;
}
dump("======= result: " + result + "\n");

to the firebug1.2 commandLine code and was able to see the following result on the debug console (not the firebug console):

======= result: Thu Apr 10 2008 16:29:50 GMT-0600 (MDT)
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WORKSFORME

Comment 9

10 years ago
re: comment 8, I was using the "expr" |new Date()|, of course.

Comment 10

10 years ago
Hmm... just checking: you know that we re-implemented the firebug 1.2 command line so that it no longer goes thru evalInSandbox (and thus new Date() works in any case).

I'm pretty sure that dolske's handy patch gets the window state correct but not the built-in objects, so for example:
new Array().constructor == [].constructor
will be false see http://code.google.com/p/fbug/issues/detail?id=559&q=label:commandline

Comment 11

10 years ago
Yes, I'm skirting that observer path.  Just used dolske's patch in firebug because it seemed like an easy way to have something interactive.  His "result" path doesn't use your observer, just the evalInSandbox route.  Here's the result of evaluating

>>> new Array().constructor == [].constructor

from the firebug command line using dolske's hack:

======= result: true

even:

>>> new Array().constructor === [].constructor
yields
======= result: true

This is real evalInSandbox, really working.  Not the observer hack.
(Reporter)

Comment 12

10 years ago
Yeah, my testcase works now (Firebug not involved). Any idea what fixed this?
Status: RESOLVED → VERIFIED
Flags: in-testsuite?

Comment 13

10 years ago
Nope, not a clue.  An anti-regression range would be nice, but I haven't got the time.
(Reporter)

Updated

8 years ago
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.