1798798 - (CVE-2023-25748) Window prompt with long description hides fullscreen notification, leads to spoof

Status: RESOLVED FIXED

(Fenix :: Browser Engine, defect, P1)

Description

[Hafiizh]

Attachment: poc.html

I found a vulnerability in firefox android where a window prompt with a long description can cover fullscreen notifications which can lead to spoofs.

steps to produce

1. open http://103.186.0.20/poc2.html or open poc.html
2. Click the open to google button
3. Then a dialog prompt window appears with a long description and covers the fullscreen notification
   4 then click ok

OS: Android 10 (Samsung M31)

i attached the poc video files.
thank you

Comment 1

[Hafiizh]

Attachment: google.jpg

Comment 2

[Hafiizh]

Attachment: video_2022-11-03_05-04-07.mp4

Comment 3

[Daniel Veditz [:dveditz]]

Dev Note: the POC shows a static fake URL bar that isn't convincing if you touch it, but that's just an implementation detail. In a real attack you could make the "url bar" as realistic-feeling as you had time to put into it.

Comment 4

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

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:cpeterson, could you consider increasing the severity of this security bug?

For more information, please visit auto_nag documentation.

Comment 5

[Chris Peterson [:cpeterson]]

sec-high P2/S2

We might need to make the alert tab-modal or change the alert's z ordering.

Comment 6

[Petru-Mugurel Lingurar [:petru]]

Wanted to confirm that this will not be solved by the patch from https://bugzilla.mozilla.org/show_bug.cgi?id=1783561.
The prompt discussed here is a dialog (has it's own Window separate from the app layout and shown on top of it) which expands to cover ~all of the available space.
A quick fix which would try to limit the number of characters shown would probably still be problematic on small screens, especially in landscape mode where the available screen height is still small.
I'd say the easiest solution (still complex) would be to migrate the dialog to a normal view inside app's layout.
With more dialogs having similar issues we would probably get to have a framework to handle them all - all in all it would be a big effort.

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Was thinking that a simpler approach may be to have the snackbar shown when entering fullscreen block further prompts until it disappears (a few seconds) or we even add an action button for it to disappear and then just queue different prompts requests.
This would ensure users are aware of the fullscreen change.

Comment 8

[Chris Peterson [:cpeterson]]

Jira issue: https://mozilla-hub.atlassian.net/browse/FXDROID-6

Comment 9

[Petru-Mugurel Lingurar [:petru]]

Saw that in such cases both Chrome Desktop/Mobile and Firefox Desktop will exit fullscreen.
Seems like a good (and easy) solution for this scenario.

Comment 10

[Chris Peterson [:cpeterson]]

P1 because Petru is working on this bug now. We should consider uplifting the fix to Beta 110.

Comment 11

[Petru-Mugurel Lingurar [:petru]]

Attachment: Support_exiting_from_prompts.patch

Exit fullscreen for any new request to show a prompt based on the account of:

• multiple prompts having the ability to block the snackbar notification
• other browsers having a similar behavior

Comment 12

[Arturo Mejia [:amejia]]

Comment on attachment 9315393 [details] [diff] [review]
Support_exiting_from_prompts.patch

Review of attachment 9315393 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!
I tested manually, with this patch after clicking the "OK" button the browser was not longer in full screen mode, and I was able to interact with it.

Comment 13

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 9315393 [details] [diff] [review]
Support_exiting_from_prompts.patch

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The patch / commit does not indicate a security issue. With a note that other browsers have the same behavior it should not raise suspicions.
• 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?: That same patch can be applied to older branches also.
• How likely is this patch to cause regressions; how much testing does it need?: Low risk. Small change.
• Is Android affected?: Yes

Comment 14

[Tom Ritter [:tjr]]

Comment on attachment 9315393 [details] [diff] [review]
Support_exiting_from_prompts.patch

Approved to land and request uplift

Comment 15

[Petru-Mugurel Lingurar [:petru]]

Patch landed in https://github.com/mozilla-mobile/firefox-android/pull/789.

Asking Chris if we should also look into uplifting this, maybe after a few days of baking in Nightly.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

This is a sec-high. We should absolutely uplift this once it's had sufficient Nightly bake time. Also note that uplifting was part of the sec-approval decision.

Comment 17

[Chris Peterson [:cpeterson]]

Petru, feel free to uplift your fix to Beta 111 whenever you're ready. As Ryan said, Beta uplift was already approved as part of the sec-approval decision.

I see that Ryan set the status flag for firefox110 = wontfix, so you don't need to uplift your fix to the 110 release branch.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

(In reply to Chris Peterson [:cpeterson] from comment #17)

Beta uplift was already approved as part of the sec-approval decision.

No, sec-approval is separate from Beta approval. We still need a backport PR with a Beta approval request before it can land.

Comment 19

[Petru-Mugurel Lingurar [:petru]]

Thank you for the support!
Uplift to 111 landed in https://github.com/mozilla-mobile/firefox-android/pull/923.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9315393 [details] [diff] [review]
Support_exiting_from_prompts.patch

Retroactively setting approval-mozilla-beta+ on this. In the future, please do request Beta approval before landing uplifts. Security approval is not the same thing.

Comment 21

[Petru-Mugurel Lingurar [:petru]]

In the future, please do request Beta approval before landing uplifts. Security approval is not the same thing.

Chris Peterson asked in https://bugzilla.mozilla.org/show_bug.cgi?id=1798798#c17 to uplift this to Beta.
I took that as an approval for the uplift but did not know of the separate approval needed for the attachment.
Noted for the next time. Thank you!

Comment 22

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 23

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 24

[Giorgio Maone [:ma1]]

While trying to backport this patch on Tor Browser for Android, we've noticed that the fix is pretty unreliable (especially on our browser based on ESR, but the problem can be reproduced on Firefox as well).

It seems that sometimes (almost always on our side) we try to exit from fullscreen before the fullscreen status is actually committed (probably because of some race condition), and therefore the mitigation fails and the "attack" succeed.

I've shipped a work-around in NoScript 11.4.19 (currently in AMO's review queue) which proxies alert/confirm/prompt and forces exitFullscreen() after the call returns, but we believe a more reliable fix is still needed upstream.

Thank you!

Comment 25

[Petru-Mugurel Lingurar [:petru]]

@Giorgio Can you offer some steps to reproduce the issue in Fenix? A video of what you're seeing would also help.
If we exit from fullscreen then the spoof described here would not happen - since the real toolbar would be shown again.
If we enter fullscreen, immediately exit and then enter again the user should see the Toast informing about the tab entering fullscreen.

Comment 26

[Giorgio Maone [:ma1]]

@Petru, on my devices I could actually (and consistently) reproduce only on our Tor Browser with the backported fix.
But my colleague Piero reported intermittent failures on Firefox as well ("Hard to reproduce, but I've managed to, a couple of times.") : I'm redirecting the needinfo to him, maybe he can provide more details.

Comment 27

[Pier Angelo Vendrame]

Hi Petru,
as Giorgio wrote, I managed to get a race condition a few times on my Pixel 4a and on an old phone I use for testing purposes.

I recorded this video: https://people.torproject.org/~pierov/bugs/41679/ScreenRecording_20230321_092447.mp4
Since the bug is still confidential, I've added a basic auth.
Username: 1798798
Password: r4c3___c0nd1t10n

As you can see in the video, the race condition is quite difficult to trigger on Firefox.
I click on the button, and then press back on the prompt just to be quicker to test, but it can happen when accepting the prompt, too.

When we get the same problem in Tor Browser, we get this message on logcat:

GeckoConsole            org.torproject.torbrowser_debug      E  [JavaScript Error: "TypeError: Not in fullscreen mode" {file: "resource://gre/modules/GeckoViewContent.jsm" line: 126}]

I haven't tried to see if I can get the same error in logcat with the latest Fenix, but I'd expect it to be there.

Comment 28

[Petru-Mugurel Lingurar [:petru]]

Thank you for these details!
So it seems that somehow we can get into the scenario of the prompt being shown but not also exiting fullscreen mode.
If a quick succession of prompts is needed to force this and the quick dismissal of the prompt is the source of this scenario I would say that the security impact is small since the user would see the prompt about the site entering in fullscreen after the quick prompt dismissal.
Fenix also moved to show a Toast instead of a Snackbar for the fullscreen message (similar to Chrome also) which would always inform the user about the tab entering in fullscreen mode so to me this scenario seem to be more of a UX issue - an inconsistent behavior regarding exiting fullscreen for specific prompts.
Opened bug 1823859 for more investigations.

Comment 29

[Pier Angelo Vendrame]

(In reply to Petru-Mugurel Lingurar [:petru] from comment #28)

If a quick succession of prompts is needed to force this and the quick dismissal of the prompt is the source of this scenario I would say that the security impact is small since the user would see the prompt about the site entering in fullscreen after the quick prompt dismissal.

I think one could be enough, at least it is for us downstream.

For the official Fenix, I had to try several times because the race condition seems to be much more unlikely to happen (maybe better performances?).

Opened bug 1823859 for more investigations.

Could you please CC me in?

Thanks!

Comment 30

[Petru-Mugurel Lingurar [:petru]]

Done.