800864 - Add some extra tests for same-compartment Location issues

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Bobby Holley (:bholley)]

When backporting bug 720619 to ESR10, I had to reinvent some stuff because the PUNCTURE machinery didn't exist back then. So I did a simple compartment principal subsumes check that generally worked.

However, there's one rub. Because the Location object describes the outer window, not the inner, we can't just rely on the compartment principals in that case. I'm working up a testcase now.

Comment 1

[Bobby Holley (:bholley)]

Oh wait! Happy day! My belt-and-suspenders code paid off. In particular, I added code to invoke the policy enforcement trap for the wrapper even if the subsumes check succeeds:

https://hg.mozilla.org/releases/mozilla-esr10/rev/863bac88c122#l1.23

In the case of a same-compartment location wrapper, this ends up throwing, because JSID_VOID isn't cross-origin accessible. So we're saved by this line here, ironically.

> // Totally unexpected, but roll with it. This is safe.

Comment 2

[Bobby Holley (:bholley)]

Attachment: Extra tests. v1

Either way, we might as well get these tests in. Can't hurt. Flagging bz for review.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 670749 [details] [diff] [review]
Extra tests. v1

r=me

Comment 4

[Al Billings [:abillings - ex-MoCo]]

Hmm. Mis-rated based on the later comments. Suggested security rating based on size of actual issue?

Comment 5

[Andrew McCreight [:mccr8]]

Good point, Al. This doesn't change code, it just adds tests that we already pass. I guess that's kind of an audit.

Comment 6

[Andrew McCreight [:mccr8]]

Is there anything left to do here besides land the patch, Bobby? You could probably also just put up the sec review requests and branch requests at the same time, as it should be okay for both (assuming you are sure it passes everywhere).

Comment 7

[Bobby Holley (:bholley)]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> Is there anything left to do here besides land the patch, Bobby? You could
> probably also just put up the sec review requests and branch requests at the
> same time, as it should be okay for both (assuming you are sure it passes
> everywhere).

IMO there's not really much to do here. The issue as-filed is invalid, but I wrote some useful tests in the mean time, which I'll land when I land all the other tests that are currently waiting for 16.0.2.

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 670749 [details] [diff] [review]
Extra tests. v1

Now that the Location stuff has blown over, requesting sec-approval to land this patch on inbound. This should pass on all our releases.

Comment 9

[Daniel Veditz [:dveditz] back 2026-04-08]

Comment on attachment 670749 [details] [diff] [review]
Extra tests. v1

sec-approval+ -- the majority of our users are on 16.0.2 now.

Comment 10

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/38a111dc16e2

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/38a111dc16e2