Assert compartments don't contain both system/non-system realms

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
Last year
2 months ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(2 attachments)

We can release-assert this in NewRealm and do some minor JS::SetRealmPrincipals refactoring so we can remove Realm::setIsSystem.

Then we can rely on this in js::IsSystemCompartment (and un-deprecate that) and use IsSystemCompartment in xpc::AccessCheck::isChrome to eliminate more JS_GetCompartmentPrincipals calls.
Comment on attachment 8999214 [details] [diff] [review]
Part 1 - Assert compartments don't contain both system/non-system realms

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

::: js/src/jsfriendapi.cpp
@@ +178,5 @@
>      if (principals == realm->principals())
>          return;
>  
> +    // We'd like to assert that our new principals is always same-origin
> +    // with the old one, but JSPrincipals doesn't give us a way to do that.

One way to do that would be to check that they mutually subsume each other.  That's a pretty strong degree of equivalence.
Attachment #8999214 - Flags: review?(luke) → review+
(In reply to Luke Wagner [:luke] from comment #3)
> One way to do that would be to check that they mutually subsume each other. 
> That's a pretty strong degree of equivalence.

Yes, I was thinking about that too :) However this is a pre-existing comment/code and I'm just interested in the isSystem bit not changing. Thinking about it more, maybe doing the stronger check is a good idea for same-compartment-realms; I may file a follow-up next week.
Attachment #8999215 - Flags: review?(mrbkap) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f73af1f3b2c
part 1 - Assert compartments don't contain both system/non-system realms. r=luke
https://hg.mozilla.org/integration/mozilla-inbound/rev/e6b953e50fbf
part 2 - Use js::IsSystemCompartment in xpc::AccessCheck::isChrome. r=mrbkap
https://hg.mozilla.org/mozilla-central/rev/9f73af1f3b2c
https://hg.mozilla.org/mozilla-central/rev/e6b953e50fbf
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Regressions: 1557664
You need to log in before you can comment on or make changes to this bug.