1877879 - (CVE-2024-1547) Show alert/confirm/prompt dialog on another website with secure padlock displayed with iframe

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Andrew McCreight [:mccr8]]

This is my second attempt at a spot fix for bug 1791283. My previous one, bug 1863852, dealt with the second test case but not the first one. In the second test case, the page with the stale alert is in the BFCache. In the first test case, I think the issue is that the window actually trying to create the alert is itself a current window global, it has a parent window that is not, so it is still not being displayed. My fix is to add a new function WindowGlobalParent::IsActiveInTab() that is like WindowGlobalParent::IsCurrentGlobal(), but it also checks AncestorsAreCurrent to avoid this situation. It also checks the BFCache status on the top BC instead of the current BC, because Nika thought there might be issues with races (it is possible this is why my previous fix didn't work for the test case but I didn't investigate it). Hopefully this new function will do a better job at checking if a window is actually visible currently.

I wrote this based on talking with DOM: Navigation people. They said the name would more correctly be something like "isActiveInRootNavigable", but that's probably not a useful name to people writing Firefox JS code would might need this function. I did add a comment about that.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1877879 - Implement isActiveInTab and use it for PromptParent.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1877879 - Add tests for for isActiveInTab.

Comment 3

[Andrew McCreight [:mccr8]]

Before landing, I need to copy over the first (and maybe second?) test case along with some instructions so that it can be verified.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: testcase.simplified.html

There are two things to verify for this patch.

The first is that the patch did not regress bug 1863852, by checking the behavior with testcase.simplified.html.

Steps to reproduce for testcase.simplified.html:

1. Visit https://www.example.com to add site to the cache.
2. In a new tab, open the attached testcase.simplified.html
3. Click the "Spoof" button.

Expected results: The browser has navigated to https://www.example.com (as seen in the address bar), and there is no alert message dialog (the text of the alert dialog starts with "We've migrated our login page to a new URL address"). It is okay if the alert appears while the address bar has testcase.simplified.html. This was already fixed by bug 1863852, so this is expected to pass before and after my patch here.

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: testcase2.bundle.html

The second thing to verify is that this patch fixes the behavior for testcase2.bundle.html. (This is the same as testcase.bundle.html from bug 1791283, except that I removed the part where it redirects to Google at the end because I think that will make it more annoying to verify. It doesn't affect the spoof we're fixing here.)

Steps to reproduce for testcase2.bundle.html:

1. Visit https://www.example.com to add site to the cache.
2. In a new tab, open the attached testcase2.bundle.html.
3. Click the "Launch" button. This will open a new tab and switch to it.
4. In the new tab, click on "Spoof".

Expected results: The browser has navigated to https://www.example.com (as seen in the address bar), and there is no alert message dialog (the text of the alert dialog starts with "We've migrated our login page to a new URL address"). It is okay if the alert appears while the address bar has testcase2.bundle.html.

Unpatched, the browser will have https://www.example.com in the address bar and there will be an alert (as described under expected results).

Comment 6

[Andrew McCreight [:mccr8]]

Comment on attachment 9377548 [details]
Bug 1877879 - Implement isActiveInTab and use it for PromptParent.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Probably not super hard. It is making a check stronger and you could compare the old and new behavior. I have a second patch in this bug with tests, but I won't land that until later.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: It shouldn't be a problem. This adds a new function that strengthens a check.
• How likely is this patch to cause regressions; how much testing does it need?: I don't think it is too worrying, but we're dealing with a lot of edge cases here, and Android works differently, so there could be some issues.
• Is Android affected?: Yes

Comment 7

[Andrew McCreight [:mccr8]]

sec-bounty: this was originally submitted by sourc7 in bug 1791283. Hopefully this attempt will actually fix everything reported there (thanks Dan for catching that in the other spot fix...) and then we could move the bounty to this bug or something.

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9377548 [details]
Bug 1877879 - Implement isActiveInTab and use it for PromptParent.

approved to land and request uplift

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f853c322b4b
Implement isActiveInTab and use it for PromptParent. r=smaug,Gijs

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9f853c322b4b

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:mccr8, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox123 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Andrew McCreight [:mccr8]]

Attachment: Bug 1877879 - Implement isActiveInTab and use it for PromptParent.

Original Revision: https://phabricator.services.mozilla.com/D200245

Comment 13

[Phabricator Automation]

Uplift Approval Request

• Needs manual QE test: yes
• Explanation of risk level: the patch might make us not open window dialogs when we should if I did something wrong
• String changes made/needed: none
• Is Android affected?: no
• Steps to reproduce for manual QE testing: see comment 4 and 5
• Fix verified in Nightly: no
• Risk associated with taking this patch: low
• Code covered by automated testing: yes
• User impact if declined: sec-high

Comment 14

[Andrew McCreight [:mccr8]]

Attachment: Bug 1877879 - Implement isActiveInTab and use it for PromptParent.

Original Revision: https://phabricator.services.mozilla.com/D200245

Comment 15

[Phabricator Automation]

Uplift Approval Request

• User impact if declined: sec-high
• Code covered by automated testing: yes
• Risk associated with taking this patch: low
• Fix verified in Nightly: no
• Is Android affected?: no
• Steps to reproduce for manual QE testing: see comment 4 and comment 5
• String changes made/needed: none
• Explanation of risk level: the patch might make us not open window dialogs when we should if I did something wrong
• Needs manual QE test: yes

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/c1ef3ef6dd3f

Comment 17

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/b4a3b229963b

Comment 18

[Daniel Veditz [:dveditz]]

As with bug 1863852 comment 17 we are not paying the bounty here in this fix because we already are tracking and awarded a bounty to the reporter's original issue in bug 1791283

Comment 19

[Andrew McCreight [:mccr8]]

Ah, sorry, I didn't notice that the bounty had actually been paid in that bug.

Comment 20

[Ciprian Georgiu, Desktop QA]

I have reproduced this issue with Firefox 119.0.1, under macOS 13, using STR from comment 4 and comment 5.

The both scenarios are verified as fixed on the latest builds, Esr 115.8.0, Beta 123.0b8 and Nightly 124.0a1. Tested with Win 11, Ubuntu 20.04 x64 and macOS 13.

Comment 21

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

a month ago, tjr placed a reminder on the bug using the whiteboard tag [reminder-test 2024-04-02] .

mccr8, please refer to the original comment to better understand the reason for the reminder.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/795c576c93f8
Add tests for for isActiveInTab. r=smaug

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/795c576c93f8