Last Comment Bug 307980 - Security error when using evalInSandbox on
: Security error when using evalInSandbox on
: fixed1.8
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- major (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Phil Schwartau
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2005-09-10 22:55 PDT by Aaron Boodman
Modified: 2006-03-12 18:54 PST (History)
12 users (show)
asa: blocking1.8b5+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase that demonstrates the issue (2.75 KB, text/plain)
2005-09-10 22:57 PDT, Aaron Boodman
no flags Details
Allow windows too (7.40 KB, patch)
2005-09-12 16:52 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Allow windows too -w (6.12 KB, patch)
2005-09-12 16:55 PDT, Blake Kaplan (:mrbkap)
brendan: review+
shaver: superreview+
asa: approval1.8b5+
Details | Diff | Splinter Review

Description Aaron Boodman 2005-09-10 22:55:30 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

I get a security error when trying to access window.alert through a sandbox with
evalInSandbox on, but not on other domains.

The sandbox's codebase is set to the URL of _content. And I am accessing alert
through something like window.alert("hi") where window is sandbox.window = _content.

Here it is in code:

var box = new Components.utils.Sandbox(url);
box.window = _content;
Components.utils.evalInSandbox("window.alert('hello from: ' +
window.location.href)", box);

Reproducible: Always

Steps to Reproduce:
1. Drop the attachment into <installdir>/components/ and have Firefox autoregister
2. You should get alerts with the URL of every non-chrome window loaded
3. navigate to, note alert
4. navigate to, note alert
5. navigate to, note no alert from (the one from
doubleclick doesn't count, we're looking for the one from the main page, not the
ad iframe). open JS console and note the security error.

Actual Results:  
Security error in JS console:

'Permission denied to get property Window.alert'

Expected Results:  
alert box with: "hello from"

I'm putting this as Major because it prevents Greasemonkey from working properly
on all sites. Its major to me anyway.
Comment 1 Aaron Boodman 2005-09-10 22:57:30 PDT
Created attachment 195602 [details]
testcase that demonstrates the issue

forgot the testcase attachment when i filed the bug
Comment 2 Aaron Boodman 2005-09-10 23:38:33 PDT
Felt like I should add the entire text of the error from the console incase it's
not easy to reproduce using my testcase:

Error: [Exception... "Component returned failure code: 0x80004003
(NS_ERROR_INVALID_POINTER) [nsIDOMLocation.replace]"  nsresult: "0x80004003
(NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
file:///C:/PROGRA~1/DEERPA~3/components/ypSandboxTestCase.js :: <TOP_LEVEL> ::
line 34"  data: no]
Source File: file:///C:/PROGRA~1/DEERPA~3/components/ypSandboxTestCase.js
Line: 34
Comment 3 Aaron Boodman 2005-09-11 01:25:23 PDT
Actually, doesn't work on yahoo either - same problem. Looks like also
sets it's document.domain property.
Comment 4 Brendan Eich [:brendan] 2005-09-11 23:31:57 PDT
mrbkap, this one looks like yours, at a glance.

Comment 5 Blake Kaplan (:mrbkap) 2005-09-12 16:02:32 PDT
It's currently impossible to do what you want in the face of setting
document.domain. To fix this, we're extending the interface of evalInSandbox to
allow you to pass a window instead of a URI, which will have all of the right
principals on it.

That would make the interesting bit in the testcase:

var sandbox = new Components.utils.Sandbox(webProgress.DOMWindow);
Comment 6 Blake Kaplan (:mrbkap) 2005-09-12 16:52:49 PDT
Created attachment 195822 [details] [diff] [review]
Allow windows too

This allows you to pass a DOMWindow to the Sandbox constructor. diff -w for
review in a second.
Comment 7 Aaron Boodman 2005-09-12 16:55:26 PDT
That solution is totally fine with me.
Comment 8 Blake Kaplan (:mrbkap) 2005-09-12 16:55:40 PDT
Created attachment 195823 [details] [diff] [review]
Allow windows too -w
Comment 9 Brendan Eich [:brendan] 2005-09-12 17:07:34 PDT
Comment on attachment 195823 [details] [diff] [review]
Allow windows too -w

r=me if you use do_QueryWrappedNative instead of rolling your own, as you
pointed out to me!

Comment 10 Blake Kaplan (:mrbkap) 2005-09-13 01:06:24 PDT
Comment on attachment 195823 [details] [diff] [review]
Allow windows too -w

>+                nsCOMPtr<nsIDOMWindow> win = do_QueryInterface(wrapper->Native());
>+                CallQueryInterface(win, &sop);

Note to self: CallQueryInterface isn't null-safe, so this needs to check |win|.
Comment 11 Mike Shaver (:shaver -- probably not reading bugmail closely) 2005-09-13 14:03:47 PDT
Comment on attachment 195823 [details] [diff] [review]
Allow windows too -w

Comment 12 Blake Kaplan (:mrbkap) 2005-09-13 15:31:42 PDT
Fix checked into trunk.
Comment 13 Blake Kaplan (:mrbkap) 2005-09-14 14:44:53 PDT
Fix checked into MOZILLA_1_8_BRANCH.

Note You need to log in before you can comment on or make changes to this bug.