Closed Bug 1823316 (CVE-2023-6870) Opened 2 years ago Closed 1 year ago

open toast using thread running in background will suppress fullscreen notification toast on Firefox and Focus

Categories

(Fenix :: General, defect, P2)

defect

Tracking

(firefox111 unaffected, firefox112 wontfix, firefox113 wontfix, firefox114 wontfix, firefox115 wontfix, firefox117 wontfix, firefox118 wontfix, firefox119 wontfix, firefox120 wontfix, firefox121 fixed)

RESOLVED FIXED
121 Branch
Tracking Status
firefox111 --- unaffected
firefox112 --- wontfix
firefox113 --- wontfix
firefox114 --- wontfix
firefox115 --- wontfix
firefox117 --- wontfix
firefox118 --- wontfix
firefox119 --- wontfix
firefox120 --- wontfix
firefox121 --- fixed

People

(Reporter: sas.kunz, Assigned: towhite)

References

(Regression)

Details

(5 keywords, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main121+])

Attachments

(12 files, 2 obsolete files)

I found a vulnerability where a fullscreen notification toast could be suppressed by a toast running on a background thread

  1. Install poc.apk and run poc
  2. open http://103.186.0.20/fullscreenbkp.html
  3. Click on "Open Fullscreen Portrait" button or "Open Fullscreen Landscape" button
    4 when click open full screen portrait it should appear fullscreen notification toast but it is not, because it has been suppressed by the toast running in the background using a thread, and when forcing the poc.apk application to die then a fullscreen notification toast appears
Attached file poc.apk
Attached file MainActivity.java
Attached file activity_main.xml
Attached file AndroidManifest.xml

i tested on Samsung M31 (Android 10)

Group: firefox-core-security → mobile-core-security
Component: Security → General
Product: Firefox → Fenix

If you can get a user to install an app, can't that app just do whatever spoofy thing without needing the browser spoof?

Attachment #9323842 - Attachment mime type: application/octet-stream → text/plain

Maybe a non malicious app could accidentally interfere, too.

Flags: needinfo?(petru.lingurar)

Sounds like a regression from bug 1816059 if we weren't using these toasts before?

Keywords: regression
Regressed by: CVE-2023-29534

(In reply to Andrew McCreight [:mccr8] from comment #9)

Maybe a non malicious app could accidentally interfere, too.

Yes Non malicious app could accidentally interfere

Interesting approach.
This to me seems to be a bug in Android and I found here [1] this scenario of showing multiple toasts in a loop being mentioned.
Testing on Android 12 and Android 13 I see the app is not affected (the fullscreen messages are showing as expected) (hinting that the Android bug was solved in the meantime) but on an Android 8 device the message does not appear. Haven't tested on more platforms yet.
Since both us (Fenix) and another application would use the same system Toast that other application can effectively starve us by it cycling in a loop through new messages to show.
Not sure why an application would do that other than with ill intent but this is a scenario that affects us and we'd need to avoid it.
I'd expect almost all applications to be affected by this system bug but I'm saying "almost all" because it seems like Chrome has been updated recently to avoid it (Chrome 100 is affected but Chrome 111 is not).
Will look into fixing this for Fenix also.

[1] https://www.wired.com/story/hack-brief-patch-your-android-phone-to-block-an-evil-toast-attack/

Assignee: nobody → petru.lingurar
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(petru.lingurar)

hi petru , i tested it on android 10. after i read the article you provided, it has something to do with tapjacking i also just read about this on https://developer.android.com/topic/security/risks/tapjacking?hl=en#risk_custom_toast

Doesn't seem like we can keep using a Toast and ensure it will always be shown.
While the Toast would help show the fullscreen message on top of anything in our app fixing all other related spoofing scenarios it being a system functionality that on older Android versions allows for background apps also to show a Toast at the same time with us makes that API a choke point for our messages.
To reconcile showing a message on top of anything in our app while also being able to style that message and having full control over it, also based on recent changes in Chrome[1] which addressed this exact issue it seems like we should show it in a new Window.
A new Window shown after various prompts (shown in their own Window on top of anything in the app) would ensure the message Window will be shown on top of the the current Activity and any other Windows. We could even show the fullscreen snackbar here and so get everything we wanted.
I won't have time to work on this. Tagging Joe to find a suitable owner.

[1] https://source.chromium.org/chromium/chromium/src/+/main:chrome/android/java/src/org/chromium/chrome/browser/fullscreen/FullscreenToast.java

Flags: needinfo?(jmahon)
Assignee: petru.lingurar → nobody
Status: ASSIGNED → NEW

Looks like crbug 1406120 was fixed in Chrome 110 (referenced in the code link above). That PoC used the Android text selection widget to cause problems so the fact that this bug was fixed might be wholly accidental.

Severity: -- → S3
Priority: -- → P2
Summary: open toast using thread running in background will suppressed fullscreen notification toast on Firefox Android → open toast using thread running in background will suppress fullscreen notification toast on Firefox Android

Does this bug also affect Focus?

this bug also affect focus i have tested on 113.0a1-202315151641

Summary: open toast using thread running in background will suppress fullscreen notification toast on Firefox Android → open toast using thread running in background will suppress fullscreen notification toast on Firefox and Focus

hello any updates?

Adding Bug 1827647 as a see-also because it seems like there are multiple potential vulnerabilities to balance with regards to fullscreen notifications.

:hafizh we had some recent organizational changes that delayed this; but we're pulling it into our upcoming sprint to pick up Petru's work where he left off.

Flags: needinfo?(jmahon)

[Security approval request comment]
How easily could an exploit be constructed based on the patch? There is no clear indication about scenarios to exploit. This will likely look like a UX decision.

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 versions => 112 (from https://bugzilla.mozilla.org/show_bug.cgi?id=1816059 which include the previous fix)

If not all supported branches, which bug introduced the flaw? https://bugzilla.mozilla.org/show_bug.cgi?id=1816059

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? No. Given how small the patch is, the same code can be uplifted.

How likely is this patch to cause regressions; how much testing does it need? Small, straightforward change. Don't expect any issue from it.

Assignee: nobody → towhite
Attachment #9349867 - Flags: sec-approval?
Attachment #9349867 - Flags: review?(jonalmeida942)

My approach to this fix is to revert the Toast added in the original fix and change the view owner of the Snackbar to the highest level fragment, e.g. a full screen DialogFragment will now be the parent to host the Snackbar

Comment on attachment 9349867 [details] [diff] [review]
Bug_1823316_-_Use_Snackbar_to_notify_on_making_app_full-screen.patch

Next time please wait for the patch to be reviewed before requesting sec-approval. But this looks OK to land if it passes review and doesn't un-fix all the previous bugs that were fixed by the toast approach.

Attachment #9349867 - Flags: sec-approval? → sec-approval+

Sure thing David.

Unfortunately, this initial patch will regress https://bugzilla.mozilla.org/show_bug.cgi?id=1821906. I have been looking at an alternative method that would involve displaying the Snackbar in it's own view above all other displayed views.

NB Chrome (& Brave) set a good precedent handling full screen mode (this is captured in comments from Edgar, Pertu & Emilio on 1823316 & 1816059). Perhaps we should look to follow a similar UX flow.

I am on PTO next week, :jonalmeida is planning on continuing work on this during this time.

Assignee: towhite → jonalmeida942

hello any updates?

Hi :Hafiizh I am continuing to look into this issue. Unfortunately the initial 'fix' caused a regression.

This bug is much less a risk for our users than the bugs we fixed that led to this regression. The others were a broad class of spoofing that attack pages could do all on their own. This one requires a malicious site to first convince the user to install a malicious app that will keep this toast busy when a web site can signal it's about to try this attack (if the toast is busy 100% of the time users will probably notice!).

I'm not even worried about that as a likely attack vector. Not only is the scenario hard to pull off, the malicious app all on its own could pull off more powerful attacks than fullscreen spoofing in a web browser so why would it bother? I think the 'sec-moderate' rating was copied over from the referenced Chrome bug, but that one did not require a confederate malicious application.

It -is- possible that some other app could inadvertently interfere with our toast in this way. We should fix this bug so that those users get the messages we intend for them, but it doesn't seem likely to happen often.

If we do switch away from using the Android system toast, consider ways to make this message more noticeable. Many modern phones are so tall that users will easily miss a static message at the bottom of the screen if there's something interesting going on at the top. And sometimes the system-chosen colors get lost on the background. If we create our own widget we can adjust the placement (closer to center?) and add attention-grabbing and harder-to-hide movement as on desktop.

Keywords: sec-moderatesec-low

Thank you for the update :dveditz.

I’ve had a few more thoughts on a non-regressing 'quick fix' for this. In the short term, we could utilize a custom AlertDialog which can get us close to how the original Snackbar implementation worked. It also provided us some other possibilities, such as:

  • require the user to ‘accept’ (the Dialog) that the app is now in full screen (e.g. it won't automatically close itself)
  • as mentioned by :dveditz change the positioning/style etc

Based on the above, I've created a prototype using a custom DialogFragment styled as a FenixSnackbar which has almost the same behaviour. Changes of note:

  • to prevent any regression - other Dialogs that could overlay the Alert are not shown if the full screen alert is displayed

Update: Remove fixed items from 'Changes of note'

Assignee: jonalmeida942 → towhite

I don't think we can have a non-closing warning. Too many people watch a lot of videos and that extra required touch/click will degrade the experience compared to our competitors. But having it stick around a bit longer than it currently does (maybe with a dismiss button for the impatient people?) could help. Motion and putting the warning closer to where people are looking would help more though (IMHO).

Fullscreen requires a user activation. Could we put the notice near where the user touched (still nicely horizontally centered)? Not too close to the top or bottom because that would look odd—say (handwaving) between 25% and 75% of the screen height. If that's weird, complicated, or ugly, it would still be an improvement to have three fixed positions (picked on aesthetics) for whether the activation was closest to top, bottom, or middle.

There might be UX research / knowledge that could help us best address this problem. We should seek their advice

Chris: is that something you can help arrange?

Flags: needinfo?(cpeterson)

Chris: is that something you can help arrange?

Yes, the Android team is planning to evaluate the fullscreen UX with our UX designers in Q4 and try to address all of the fullscreen notification corner cases. We will definitely include your suggestions here and can include you in the design review.

Flags: needinfo?(cpeterson)
Attached patch full_screen_notification_2.patch (obsolete) — Splinter Review
Attachment #9349867 - Attachment is obsolete: true
Attachment #9349867 - Flags: review?(jonalmeida942)
Attachment #9360989 - Flags: review?(jonalmeida942)
Comment on attachment 9360989 [details] [diff] [review] full_screen_notification_2.patch Review of attachment 9360989 [details] [diff] [review]: ----------------------------------------------------------------- This change looks good to me, thanks for putting it together Tom! We also spoke offline and Tom suggested we ask QA to verify that we aren't regressing any of our other sec bugs that are linked under this meta. ::: android-components/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/dialog/FullScreenNotificationImpl.kt @@ +35,5 @@ > + * [DialogFragment] that is configured to match the style and behaviour of a Snackbar. > + * > + * @property layout the layout to use for the dialog. > + */ > +class FullScreenNotificationImpl(@LayoutRes val layout: Int) : nit: `Impl` doesn't really tell us much about the thing (we try to avoid it usually). Maybe something like `FullScreenNotificationDialog`, `FullScreenNotificationDialogFragment`, or `FullScreenNotificationFragment`?
Attachment #9360989 - Flags: review?(jonalmeida942) → review+
Attachment #9360989 - Attachment is obsolete: true

Hi :miralobontiu, could QA provide assistance in verifying this patch against the following potential regressions please?

I cannot add the Fenix & Focus apk's here as they are too large, I will find an alternative.

Flags: needinfo?(mlobontiuroman)

Verified on the Fenix & Focus Nightly provided builds, and it seems that there are no regressions.
We've verified all the issues mentioned on Comment 40.
Tested with Google Pixel 6 (Android 14), and Samsung Galaxy A53 (Android 13).

Flags: needinfo?(mlobontiuroman)

Comment on attachment 9360989 [details] [diff] [review]
full_screen_notification_2.patch

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: It's clear that Fenix is now using a 'full screen notification' and not displaying the site permissions dialog if the 'full screen notification' is displayed. However, as already noted this is fairly low risk and any exploit against Toast would still require the additional 'Toast blocker' app to be installed
  • 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?: 112
  • If not all supported branches, which bug introduced the flaw?: Bug 1816059
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?:
  • How likely is this patch to cause regressions; how much testing does it need?: Testing carried out by QA
  • Is Android affected?: No
Attachment #9360989 - Flags: sec-approval?
Attachment #9361436 - Flags: sec-approval?

Comment on attachment 9360989 [details] [diff] [review]
full_screen_notification_2.patch

Because this is just a sec-low sec-approval is not required. You can proceed to land this when ready

Attachment #9360989 - Flags: sec-approval?
Attachment #9361436 - Flags: sec-approval?
Status: NEW → RESOLVED
Closed: 1 year ago
Flags: qe-verify+
Resolution: --- → FIXED
Target Milestone: --- → 121 Branch

No need to uplift to Beta 120. This fix can ride the trains with 121.

Flags: qe-verify+
Group: mobile-core-security → core-security-release
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main121+]

How is the decision about the bounty? because the sec-bounty flag has not been set

Flags: needinfo?(dveditz)
Alias: CVE-2023-6870
Flags: sec-bounty?

We have decided not to award a bounty for this bug. As noted in our policy we are interested in encouraging research into more severe vulnerabilities and award bounties to sec-low bugs only when they are interesting and novel.

Flags: sec-bounty?
Flags: sec-bounty-
Flags: needinfo?(dveditz)

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter

Group: core-security-release
Flags: sec-bounty-hof+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: