1902996 - (CVE-2024-8388) New approach to fix bugs where fullscreen notification is hidden under OS permission/confirmation prompts.

Status: RESOLVED FIXED

(Firefox for Android :: General, defect, P2)

Description

[Polly [:polly]]

We have a few bugs where various system prompts overlap fenix's fullscreen notification snackbar dialog. This can lead to spoofing since the user is not aware that they are in fullscreen.
Various attempts have been made to fix this at the specific individual journey level, eg. by exiting fullscreen when a dialog is shown.
We would ideally like to have a more generalised approach to this which tackles existing issues and shows the fullscreen notification in a way that can't be overlapped by other dialogs.

Comment 1

[Polly [:polly]]

Following an investigation and various discussions, we have decided on a way forward for presenting the fullscreen notification.
The plan is:

• Modify the message used from "Entering full screen mode" to give more helpful instructions on how to exit fullscreen.
• To present the message, use a Toast instead of a custom Dialog.

Notes:

• Devices running pre-v12 versions of Android may require OS level security patches to protect them from 'toastburst' style attacks (see https://developer.android.com/privacy-and-security/risks/tapjacking#risk_custom_toast). (Previously, we moved away from using Toast for this reason, but plenty of time has now elapsed for manufacturers to offer security patches and users to apply them.)
• We should retest bugs associated with fullscreen. Toasts appear at a higher z-index than Dialogs, so we hope this change will reduce the likelihood of the fullscreen message being obscured by other prompts, but there may be situations where it is still preferable to exit fullscreen in response to some system event.

Comment 2

[Polly [:polly]]

There are two scenarios here which we can distinguish between programmatically to offer a more useful message.

a) Gesture nav is not available on the device (if it is below Android 10) or gesture nav is switched off, so the user needs to press a button to go back.
b) Gesture nav is available and switched on.

For these two situations, i would suggest showing messages something like:

a) To exit fullscreen, drag from top and press back
b) To exit fullscreen, drag from top and use back gesture

We probably want to keep these messages as short as we can whilst preserving meaning, so they aren't too intrusive. Both of these suggested messages fit onto two lines of a Toast at default font size.

Comment 3

[Polly [:polly]]

Attachment: Bug 1902996 - Improve messaging for fullscreen notifications.

Instead of explaining to users that they are in fullscreen mode, which may be obvious, we explain how to get out of it :)
Also use a Toast rather than a custom Dialog.

Comment 4

[Polly [:polly]]

Comment on attachment 9411256 [details]
Bug 1902996 - Improve messaging for fullscreen notifications.

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I think it would be very difficult to exploit the issues caused by overlapping fullscreen notifications. A malicious actor would have to construct a convincing spoof of Firefox UI across all android versions.
• 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 branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all branches and yes
• If not all supported branches, which bug introduced the flaw?: n/a
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: n/a
• How likely is this patch to cause regressions; how much testing does it need?: I think it would be good to retest the bugs in the associated metas, android-sec-fullscreen and android-system-prompt-fullscreen - at least some of them should be fixed by this patch
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 5

[Daniel Veditz [:dveditz]]

Instead of explaining to users that they are in fullscreen mode, which may be obvious, we explain how to get out of it :)

The existing message was likely intended to be a space-contrained variant of the Desktop message, but unfortunately focused on the one part of the message ("now fullscreen") that was incidental:

permission.site is now fullscreen [Exit Full Screen (esc)]

Our main focus was

1. Which site was fullscreen
2. How to get out

"you are fullscreen" wasn't the point, but WHICH site. The user's assumption is naturally that it's the site they were just looking at, but with nested frames and ads it might not be who you expect! It might have been contrary to the intent of the top-most site even. This was in fact abused, so it was a very important piece of information. The abuse got bad enough that browsers agreed on a web-breaking spec change: framed content could now only use fullscreen if the parent site granted permission by adding the allowfullscreen attribute to the <iframe>. This concept was later generalized into the Permission Policy spec, so allow="fullscreen" is preferred.

With Permission Policy the parent context is taking responsibility for what the nested frames do, so the original assumption by users that the fullscreen action is part of "the site they're on" is essentially true, even if the specific content is served from a 3rd party. The site name in the fullscreen notice was changed to the top-most site to match the prompts for other delegated permissions, so it no longer carries much meaning beyond "you're in fullscreen"

We should probably update the desktop string, too. But that would be a different bug obviously.

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 9411256 [details]
Bug 1902996 - Improve messaging for fullscreen notifications.

sec-approval+ = dveditz
Given the string changes I think this has to "ride the trains" and can't jump to beta to fix the dependent bugs any earlier. That should be fine: gives us more time to test.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d76a4fa87a2
Improve messaging for fullscreen notifications. r=android-reviewers,gl

Comment 8

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3007f9376ee1
Backed out changeset 9d76a4fa87a2 for causing lint-fenix failures. CLOSED TREE

Comment 9

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

Backed out for causing lint-fenix failures:
https://hg.mozilla.org/integration/autoland/rev/3007f9376ee168d3e8a2489adcebf2b41b9499d5

Push with failures
Failure log

[task 2024-07-16T09:56:34.813Z] > Task :app:lintReportFenixDebug
[task 2024-07-16T09:56:34.813Z] Wrote HTML report to file:///builds/worker/checkouts/gecko/mobile/android/fenix/app/build/reports/lint-results-fenixDebug.html
[task 2024-07-16T09:56:34.913Z] 
[task 2024-07-16T09:56:34.913Z] > Task :app:lintFenixDebug FAILED
[task 2024-07-16T09:56:34.913Z] Lint found 2 errors, 3 warnings (9 errors, 173 warnings filtered by baseline lint-baseline.xml). First failure:
[task 2024-07-16T09:56:34.913Z] 
[task 2024-07-16T09:56:34.913Z] /builds/worker/checkouts/gecko/mobile/android/fenix/app/src/main/res/layout/full_screen_notification_dialog.xml:6: Error: The resource R.layout.full_screen_notification_dialog appears to be unused [UnusedResources]
[task 2024-07-16T09:56:34.913Z] 
[task 2024-07-16T09:56:34.913Z] The full lint text report is located at:
[task 2024-07-16T09:56:34.913Z]   /builds/worker/checkouts/gecko/mobile/android/fenix/app/build/intermediates/lint_intermediate_text_report/fenixDebug/lintReportFenixDebug/lint-results-fenixDebug.txt
[task 2024-07-16T09:56:35.013Z] 
[task 2024-07-16T09:56:35.013Z] FAILURE: Build failed with an exception.
[task 2024-07-16T09:56:35.013Z] 
[task 2024-07-16T09:56:35.013Z] * What went wrong:
[task 2024-07-16T09:56:35.013Z] Execution failed for task ':app:lintFenixDebug'.
[task 2024-07-16T09:56:35.013Z] > Lint found errors in the project; aborting build.
[task 2024-07-16T09:56:35.013Z]   
[task 2024-07-16T09:56:35.013Z]   Fix the issues identified by lint, or add the issues to the lint baseline via `gradlew updateLintBaseline`.
[task 2024-07-16T09:56:35.013Z]   For more details, see https://developer.android.com/studio/write/lint#snapshot
[task 2024-07-16T09:56:35.013Z]   
[task 2024-07-16T09:56:35.013Z]   Lint found 2 errors, 3 warnings (9 errors, 173 warnings filtered by baseline lint-baseline.xml). First failure:
[task 2024-07-16T09:56:35.013Z]   
[task 2024-07-16T09:56:35.013Z]   /builds/worker/checkouts/gecko/mobile/android/fenix/app/src/main/res/layout/full_screen_notification_dialog.xml:6: Error: The resource R.layout.full_screen_notification_dialog appears to be unused [UnusedResources]
[task 2024-07-16T09:56:35.013Z]   
[task 2024-07-16T09:56:35.013Z]   The full lint text report is located at:
[task 2024-07-16T09:56:35.013Z]     /builds/worker/checkouts/gecko/mobile/android/fenix/app/build/intermediates/lint_intermediate_text_report/fenixDebug/lintReportFenixDebug/lint-results-fenixDebug.txt

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae6cdc11bab6
Improve messaging for fullscreen notifications. r=android-reviewers,gl

Comment 11

[Polly [:polly]]

fixed the lint errors now and relanded... also raised a bug to cover why the linter wasn't running on try!

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/ae6cdc11bab6

Comment 13

[Donal Meehan [:dmeehan]]

Setting Fx129 to wontfix based on Comment 6

Comment 14

[Daniel Veditz [:dveditz]]

For completeness, we switched to a Snackbar in Firefox 121 (bug 1823316) to avoid a problem with Toasts on old versions of Android. It turns out it's better to live with that edge-case (rapidly disappearing as old phones are replaced) than the problems introduced by the Snackbar.

Comment 15

[Daniel Veditz [:dveditz]]

Attachment: advisory.txt