1338313 - Sort out how content-principal sandboxes should behave when script is disabled

Status: REOPENED

(Core :: XPConnect, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

If I set the "javascript.enabled" pref to false, and then create a content-principal sandbox, I can evaluate script in it just fine and defined functions whose global is the sandbox global.  But xpc::Scriptability::Get(sandboxFunction).Allowed() returns false.  That's because Scriptability::Scriptability only cares about the principal and our principal is some random content principal.  

This doesn't block evaluation of script in the sandbox, because xpc::EvalInSandbox uses JS::Evaluate directly, not via something like nsJSUtils that would check xpc::Scriptability::Get().Allowed().  But if that script tries to call APIs that take web idl callbacks, and passes functions from inside the sandbox, those functions will not run.

This seems pretty dubious to me.  We should probably be consistent here, right?  Not sure in which direction.

Comment 1

[Bobby Holley (:bholley)]

Yeah, seems like xpc::EvalInSandbox should check scriptability just like everything else.

Comment 2

[Tom S]

What about WebExtensions? We already have bug 1329731.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Tom Schuster [:evilpie] from comment #2)
> What about WebExtensions? We already have bug 1329731.

Web extensions run with nsExpandedPrincipal, right? Aren't those exempt from the script disabling machinery?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I think web extension content scripts run with an nsExpandedPrincipal.

But the actual web extension code apparently does not...

Comment 5

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #4)
> I think web extension content scripts run with an nsExpandedPrincipal.
> 
> But the actual web extension code apparently does not...

What does it run with - a null principal? We should just add a SandboxOption or something if we don't already have a similar opt-out mechanism.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> What does it run with - a null principal?

Based on the patches in bug 1329731, I'd guess a moz-extension: codebase principal.  But I'm not sure.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Yeah, seems like xpc::EvalInSandbox should check scriptability just like everything else.

So my worry about that is that it's backwards-incompatible and might break various extensions...

But yes, I guess we could add opt-in sandbox behavior here.  The worry is that no one will test in the "JS disabled" configuration and hence never set this flag.

Comment 8

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #7)
> > Yeah, seems like xpc::EvalInSandbox should check scriptability just like everything else.
> 
> So my worry about that is that it's backwards-incompatible and might break
> various extensions...

We could also just wait 6 months for that problem to go away.

Comment 9

[Andrew Overholt [:overholt]]

Given comment 8 I'm saying P3 (aka "backlog") but please lmk if this should be higher priority.

Comment 10

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.