Closed Bug 1482423 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(2 files)

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: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.