Closed Bug 1092446 Opened 5 years ago Closed 5 years ago

[e10s] Unprivileged scopes can't call content-to-chrome CPOWs

Categories

(Core :: XPConnect, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
e10s m5+ ---

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(1 file)

For Greasemonkey, we have a sandbox with expanded principals that lives in the content process. It needs to be able to reference functions in the chrome process via CPOWs. Currently this doesn't work. The problem is that the code that normally creates COWs looks like this:
http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/WrapperFactory.cpp#425

Notice how it checks (IdentifyStandardInstance(obj) == JSProto_Function). Unfortunately, CPOWs return JSProto_Proxy from IdentifyStandardInstance, so they don't get this treatment. Instead we get a totally opaque wrapper which prevents calling.

I'm not really sure how to handle this Bobby. I've included a test to make it easy to reproduce the problem. I also added kind of a crummy workaround.

It should be possible to check here if the object in the chrome process is actually passes the JSProto_Function test. We could send the result of IdentifyStandardInstance(obj) along with each CPOW. What I have here is a little easier, though, so I'm wondering if it will work.
Attachment #8515398 - Flags: feedback?(bobbyholley)
Comment on attachment 8515398 [details] [diff] [review]
greasemonkey-problem

Review of attachment 8515398 [details] [diff] [review]:
-----------------------------------------------------------------

Chrome code should _really_ be using Cu.exportFunction here - as it stands, the function objects that get exposed to content are going to look really weird (no prototype, no .call/.apply, etc). It's worth getting in touch with the GreaseMonkey folks and getting a bug on file to fix their stuff.

Nevertheless, I'll obviously resist the urge to pork-barrel my desired addon changes into e10s. This matches what we currently allow in the same-process case, so it's fine.

r=me with the below fixed.

::: js/xpconnect/wrappers/WrapperFactory.cpp
@@ +425,5 @@
>      // If this is a chrome function being exposed to content, we need to allow
>      // call (but nothing else).
>      else if (originIsChrome && !targetIsChrome &&
> +             (IdentifyStandardInstance(obj) == JSProto_Function ||
> +              (jsipc::IsCPOW(obj) && JS::IsCallable(obj))))

Please add a check here to make sure we're in the content-process. There should be no need for this in a parent process.
Attachment #8515398 - Flags: review+
Attachment #8515398 - Flags: feedback?(bobbyholley)
Attachment #8515398 - Flags: feedback+
The rooting hazards that this patch was implicated in did not go away after the backout. Relanding.
https://hg.mozilla.org/integration/mozilla-inbound/rev/22047daf1460
https://hg.mozilla.org/mozilla-central/rev/22047daf1460
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.