Closed Bug 1151385 Opened 9 years ago Closed 9 years ago

Fail earlier when a non-subsumed object is passed as sandboxPrototype

Categories

(Core :: XPConnect, defect)

37 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: warren.r.bank, Assigned: bholley)

Details

Attachments

(3 files)

Attached file error - CU Sandbox.txt
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150122214805

Steps to reproduce:

The file attachment contains 3 variations on a minimal test case, which illustrates (in greater detail) how the following pseudo-code will no-longer work under FF 37:

var sandbox, name, val;
sandbox = new Cu.Sandbox(null_principal, sandbox_options);
name = 'myVariable';
val = {"a":1};
sandbox[name] = Cu.cloneInto(val, sandbox);


Actual results:

Error: Permission denied to access property "myVariable"


Expected results:

per the docs:
  * https://developer.mozilla.org/en-US/docs/Components.utils.Sandbox
  * https://developer.mozilla.org/en-US/docs/Components.utils.cloneInto

the variable "myVariable" should be scoped within the sandbox, and available to code that is evaluated via:
    Cu.evalInSandbox(code, sandbox)

{request}:
========
I apologize if it's bad form to specifically request help from a particular developer. But I've discussed Sandbox topics in the past with Bobby Holley @bholley ..and his knowledge of the topic is impressive. If he's available and wouldn't mind a quick look, his help would be hugely appreciated. Thanks all :)
Pinging bholley, as requested in the report.

Warren, you might want to try to discuss this on irc.mozilla.org (#developers) or in dev-platform mailing list, in bugzilla you run the risk of being unnoticed, since there are a lot of bug reports.
Component: Untriaged → XPConnect
Flags: needinfo?(bobbyholley)
Product: Firefox → Core
The problem is that you're passing sandboxPrototype: {}. The {} will evaluate to an object in the caller's scope (chrome), which is opaque to the null-principaled sandbox. This is going to make any code running in the sandbox throw every time it does a global property lookup. But as you noticed, sandbox globals are immune to Xrays, so you actually see the same behavior from the caller when accessing a property.

So the code in comment 0 is wrong, but I agree that the failure mode here is really confusing. I'll attach a patch to fail earlier.

Thanks for the bug report!
Flags: needinfo?(bobbyholley)
Attachment #8600717 - Flags: review?(gkrizsanits)
Following the suggestion made by Bobby Holley, I attached a 4th/final variation of the test case that now runs correctly.

So far as I'm concerned, this case is closed;
As always, I really appreciate the help!
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
(In reply to Warren Bank from comment #5)
> So far as I'm concerned, this case is closed;
> As always, I really appreciate the help!

No problem!

I'm repurposing this bug for the attached patch, so reopening.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Summary: [v37.0.1] cannot assign object variables into an instance of Cu.Sandbox → Fail earlier when a non-subsumed object is passed as sandboxPrototype
Assignee: nobody → bobbyholley
Attachment #8600717 - Flags: review?(gkrizsanits) → review+
https://hg.mozilla.org/mozilla-central/rev/be04da4b954d
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.