This bug was filed from the Socorro interface and is report bp-9ff58ec7-b2ba-4f63-a09f-a305d2170309. ============================================================= There are 2 crashes on nightly 55 with buildid 20170308030207. In analyzing the backtrace, the regression may have been introduced by patch [1ÿ to fix bug 1314361.  https://hg.mozilla.org/mozilla-central/rev?node=73265f9e2940d06af625ee3b2bd6f57a87c932c2
:kmag, is it related to your bug fix ?
Its failing this assertion: https://hg.mozilla.org/mozilla-central/annotate/58753259bfeb/dom/base/PostMessageEvent.cpp#l115
It looks like it may be. See bug 1314361 comment 86. It looks like IsOriginAttributesEqualIgnoringAddonId was never updated to check first-party domain when that attribute and the IsOriginAttributesEqualIgnoringFPD helper were added, so it accidentally served that purpose. That check probably needs to be updated to use IsOriginAttributesEqualIgnoringFPD now instead of the IsOriginAttributesEqualIgnoringAddonId check that was removed.
Depends on: 1319773
Flags: needinfo?(kmaglione+bmo) → needinfo?(amarchesini)
Assignee: nobody → amarchesini
Attachment #8845832 - Flags: review?(tom)
So I just want to unpack this to make sure I understand it correctly. What's happening here is we're sending a postMessage event from one window to the other. The current assert says: > MOZ_DIAGNOSTIC_ASSERT(providedOrigin != targetOrigin || > (mProvidedPrincipal->OriginAttributesRef() == > targetPrin->OriginAttributesRef()), > "Unexpected postMessage call to a window with mismatched " > "origin attributes"); Which says: "The two domains that are talking to each other are not the same domain. But, if they are the same domain, then they have identical Origin Attributes." This is important to assert because we don't want windows postMessaging to each other if they're in different OA 'zones' - PBM vs non-PBM, Container A vs Container B. Are we certain this isn't possible? I guess if the only ways to get a handle are window.frames, iframe.contextWindow, window.open(), and window.[parent,top,opener] - none of those can trigger opening in a different OA 'zone', so we're safe because of that (and that alone probably...) Except... if First Party Isolation is turned on, then the different windows will have the FirstParty string filled in in OA, and the equals check will fail. We _want_ First Party Isolation to be ignored in this case because you should be able to postMessage between domains with FPI turned on. The fact that this worked before was just a bug of the (old) IsOriginAttributesEqualIgnoringAddonId function. Assuming all that's correct, I have a few questions: 1) How does this assert get triggered? The part I'm confused about is the 'providedOrigin != targetOrigin' - to continue to the OA part of the assert, the two origins have to be identical. Is this it: With FPI enabled, a.com in Window1 opens a window to b.com (Window2) - b.com makes an iframe to a.com (Window2) and passes its window.opener handle to it's iframe. So a.com(in an iframe in Window2 with FirstParty=b.com) attempts to talk to a.com(Window1 with FirstParty=a.com)? This should be allowed (but it is asserting and crashing.) 2) This check only occurs if the target principal (the principal of the receiving message) is not equal to the provided principal (the principal of the _sending_ window *at the time it was sent*). "If these two windows are the same domain, with identical Origin Attributes - why would they have different principals?" Assuming I got #1 right, is the answer "Because the Target Principal is (or includes) b.com" ? 3) Why do we let them pass if they aren't the same domain? There isn't a way, currently, for a window to get a handle to a window that is in a different OA 'zone'. But hypothetically let's imagine there was (for example we created the feature that lets all example.com windows automatically go to Container Zed). In this case, you could postMessage between containers because the domain check passes. In contrast, if we *only* asserted that the OA attributes were identical (ignoring FPI) - we should still be safe here (not getting any assertions) and we'd be a bit future-proofed. Or did I miss something?
> with FirstParty=a.com)? This should be allowed (but it is asserting and > crashing.) What do you mean with "and passes its window.opener handle to it's iframe" ? > 3) Why do we let them pass if they aren't the same domain? This is a good point. We should change the assertion to compare OAs always.
Flags: needinfo?(amarchesini) → needinfo?(tom)
(In reply to Andrea Marchesini [:baku] from comment #6) > > with FirstParty=a.com)? This should be allowed (but it is asserting and > > crashing.) > > What do you mean with "and passes its window.opener handle to it's iframe" ? So the only way to postMessage is by getting a handle to a window, right? You get a handle as the return value of window.open(), and you can also get a handle from window.opener. So if b.com(in Window2) - which was created by a window.open() call by a.com(in Window1) - has an iframe containing a.com, it could pass its window.opener value (which is a handle to a.com(in Window1) to the iframe and then the iframe could call postMessage. At least I think that's correct.
Flags: needinfo?(tom) → needinfo?(amarchesini)
STR: 1. Open a container 2. Go to: https://www.alttickets.com search for a concert and click book ticket 3. Go through cart process as guest, then pay by paypal. 4. When paypal loads, it crashes both tabs.
I also compiled Baku's patch works for the STR that I had, but then I also could not reproduce the crash when I removed it also. Maybe something else in central fixed the crash? My STR was super reproducible in Nightly 55.0a1 Ubuntu. There appears to be two cart flows for the site above, sometimes it's in an iframe which only replicates when you have purchased. It appears the iframe flow is when you are already logged in.
Comment on attachment 8845832 [details] [diff] [review] check.patch As discussed, change it to remove the domain check and just check the OA (using IsOriginAttributesEqualIgnoringFPD) always.
Attachment #8845832 - Flags: review?(tom) → review-
2 years ago
Duplicate of this bug: 1360145
2 years ago
Duplicate of this bug: 1360142
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/558841792eab Fixing an assertion about OriginAttribute comparison in PostMessage, r=tjr
You need to log in before you can comment on or make changes to this bug.