1708354 - Notifications are allowed in a non-same-origin iframe if previously granted top-level

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Veditz [:dveditz]]

According to MDN, the Notification permission can only be granted in a top-level document or same-origin iframe. I don't see any such restriction in the spec but it's consistent with permissions governed by Permission Policy (cross-origin permission prompts confuse users) and does appear to work that way in Firefox.

However if the permission is already granted we will happily let a cross-origin iframe send Notifications. Maybe no real harm since users can revoke the permission from the notifications themselves, but it's inconsistent with how permissions governed by Permission Policy work (although notifications don't seem to be covered by that spec).

Is this OK?

STR:

1. copy and paste the following url in the Firefox address bar (navigating will not work) and note there are no notifications as the costume changes: data:text/html,<iframe%20height=800%20width=400%20src="https://chrisdavidmills.github.io/emogotchi/"></iframe>
2. go to the site directly and grant the notification permission. Notifications will work.
3. open another tab on the data: URL from step 1. and note you're now getting notifications.

Comment 1

[Daniel Veditz [:dveditz]]

It's not really worse than Push Notifications: you have to grant the push permission at the top level (it's the same permission in Firefox), but then a document from that origin doesn't have to be open anywhere when the push notifications come in. Those are handled by the registered ServiceWorker, which we'll wake up if necessary.

Comment 2

[Anne (:annevk)]

I don't think this should work. Per our new partitioned thinking (tm), a top-level A1 is separate from an embedded third-party A2 and should not be able to share state, including permissions.

That raises the question what should happen if A2 got storage access, but even in that case I don't think it should get the permission to create notifications. As the user is not visiting A2 directly, A2 as third-party is an implementation detail of some first-party.

As for Permissions Policy, we did discuss notifications and decided delegation does not make sense as notifications are inherently origin-bound. For instance, if A2 were embedded in B and Permissions Policy was a thing for notifications and B did delegate notifications, A2 would request, the user would grant B (as they are the top-level and thus responsible and what we would show the user), but then A2 would create a notification for A which would mismatch with user expectations (I granted B, not A!). So because notifications are origin-bound they have to be an exclusively first-party feature that only works there.

Comment 3

[Johann Hofmann [:johannh]]

Yeah this is happening because this code for showing notifications isn't checking third-partyness, it only checks for permission for the principal. Fixing that should be simple. Leaving the needinfo on for a bit longer.

Comment 4

[Johann Hofmann [:johannh]]

FYI I filed https://github.com/whatwg/notifications/issues/177, we should probably get this right in the spec, in addition to fixing this bug.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Johann Hofmann [:johannh] from comment #3)

Yeah this is happening because this code for showing notifications isn't checking third-partyness, it only checks for permission for the principal. Fixing that should be simple. Leaving the needinfo on for a bit longer.

We want to take a look at this one and assess the possibility of the fix.

Comment 6

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

We don't allow request permission on iframe if it is not the same origin as top-level one, https://searchfox.org/mozilla-central/rev/6c1a6e3263c2cae45b285b489ef340d944102262/dom/notification/Notification.cpp#526-540. In such case, in addition to not allow showing notification, it also makes sense that permission also returns "denied".

Comment 7

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

Attachment: Bug 1708354 - Disallow showing notification from cross-origin iframes; r?smaug

We disallow notification permission requests from cross-origin iframes (see bug 1560741).
It makes sense that we also disallow showing notification from cross-origin iframes,
even if user has granted the notification permission on that origin.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a75c8ba68f39
Disallow showing notification from cross-origin iframes; r=smaug

Comment 9

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on test_notification_tag.html

• backout: https://hg.mozilla.org/integration/autoland/rev/787e7878db31b1776c7bdee7a2b74d68aa7f69d2
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=a75c8ba68f392f0db4a5c6532dd6eda8aef7520f
• failure log: https://treeherder.mozilla.org/logviewer?job_id=391658204&repo=autoland&lineNumber=3655

[task 2022-09-28T08:27:43.067Z] 08:27:43     INFO - TEST-START | dom/notification/test/mochitest/test_notification_tag.html
[task 2022-09-28T08:33:09.645Z] 08:33:09     INFO - TEST-INFO | started process screentopng
[task 2022-09-28T08:33:09.784Z] 08:33:09     INFO - TEST-INFO | screentopng: exit 0
[task 2022-09-28T08:33:09.786Z] 08:33:09     INFO - TEST-UNEXPECTED-FAIL | dom/notification/test/mochitest/test_notification_tag.html | Test timed out. - 
[task 2022-09-28T08:33:10.651Z] 08:33:10     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-09-28T08:33:10.654Z] 08:33:10     INFO - TEST-UNEXPECTED-FAIL | dom/notification/test/mochitest/test_notification_tag.html | [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once.  Make sure you use SimpleTest.waitForExplicitFinish() if you need it.) 
[task 2022-09-28T08:33:10.654Z] 08:33:10     INFO -     SimpleTest.ok@SimpleTest/SimpleTest.js:414:16
[task 2022-09-28T08:33:10.655Z] 08:33:10     INFO -     afterCleanup@SimpleTest/SimpleTest.js:1413:18
[task 2022-09-28T08:33:10.655Z] 08:33:10     INFO -     executeCleanupFunction@SimpleTest/SimpleTest.js:1489:7
[task 2022-09-28T08:33:10.655Z] 08:33:10     INFO -     SimpleTest.finish@SimpleTest/SimpleTest.js:1509:3
[task 2022-09-28T08:33:10.656Z] 08:33:10     INFO -     killTest@SimpleTest/TestRunner.js:195:22
[task 2022-09-28T08:33:10.663Z] 08:33:10     INFO - GECKO(2177) | MEMORY STAT | vsize 2544MB | residentFast 139MB | heapAllocated 7MB
[task 2022-09-28T08:33:10.679Z] 08:33:10     INFO - TEST-OK | dom/notification/test/mochitest/test_notification_tag.html | took 327611ms
[task 2022-09-28T08:33:10.728Z] 08:33:10    ERROR - TEST-UNEXPECTED-FAIL | /tests/dom/notification/test/mochitest/test_notification_tag.html logged result after SimpleTest.finish(): [SimpleTest.finish()] No checks actually run. (You need to call ok(), is(), or similar functions at least once.  Make sure you use SimpleTest.waitForExplicitFinish() if you need it.)

Comment 10

[Edgar Chen [:edgar] (Parental leave until Oct. 09.)]

https://treeherder.mozilla.org/jobs?repo=try&revision=fa31bff36d5b72c6c2a3e88b5d0cfc79e9282703&group_state=expanded

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fb669ea3b7b
Disallow showing notification from cross-origin iframes; r=smaug

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/0fb669ea3b7b

Comment 13

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest Nightly as well as Beta 107.0b2. I've also updated the flags for Release and ESR since those versions are also affected.

Comment 14

[Rares Doghi, Desktop QA]

Updating Flags to Verified.