open toast using thread running in background will suppress fullscreen notification toast on Firefox and Focus
Categories
(Fenix :: General, defect, P2)
Tracking
(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)
2.85 MB,
application/vnd.android.package-archive
|
Details | |
5.50 MB,
video/mp4
|
Details | |
980 bytes,
text/plain
|
Details | |
298 bytes,
text/xml
|
Details | |
658 bytes,
text/xml
|
Details | |
1.80 MB,
video/mp4
|
Details | |
2.15 MB,
video/mp4
|
Details | |
2.54 MB,
video/mp4
|
Details | |
2.47 MB,
video/mp4
|
Details | |
20.16 KB,
patch
|
Details | Diff | Splinter Review | |
59 bytes,
text/x-github-pull-request
|
Details | Review | |
278 bytes,
text/plain
|
Details |
I found a vulnerability where a fullscreen notification toast could be suppressed by a toast running on a background thread
- Install poc.apk and run poc
- open http://103.186.0.20/fullscreenbkp.html
- 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
this vulnerability happened after fixed from https://github.com/mozilla-mobile/firefox-android/commit/e2ae2a040d1990cbc775bec5715aebd690cbe4a3
Updated•2 years ago
|
Comment 8•2 years ago
|
||
If you can get a user to install an app, can't that app just do whatever spoofy thing without needing the browser spoof?
Updated•2 years ago
|
Comment 9•2 years ago
|
||
Maybe a non malicious app could accidentally interfere, too.
Updated•2 years ago
|
Comment 10•2 years ago
|
||
Sounds like a regression from bug 1816059 if we weren't using these toasts before?
Reporter | ||
Comment 11•2 years ago
|
||
(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
Comment 12•2 years ago
|
||
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/
Reporter | ||
Comment 13•2 years ago
|
||
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
Updated•2 years ago
|
Comment 14•2 years ago
|
||
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 Window
s. 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.
Updated•2 years ago
|
Comment 15•2 years ago
|
||
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.
Updated•2 years ago
|
Updated•2 years ago
|
Comment 17•2 years ago
|
||
Does this bug also affect Focus?
Reporter | ||
Comment 18•2 years ago
|
||
this bug also affect focus i have tested on 113.0a1-202315151641
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Reporter | ||
Comment 19•2 years ago
|
||
hello any updates?
Comment 20•2 years ago
|
||
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.
Updated•2 years ago
|
Updated•2 years ago
|
Comment hidden (duplicate) |
Comment hidden (duplicate) |
Assignee | ||
Comment 23•1 years ago
|
||
Assignee | ||
Comment 24•1 years ago
|
||
Assignee | ||
Comment 25•1 years ago
|
||
Assignee | ||
Comment 26•1 years ago
|
||
Assignee | ||
Comment 27•1 years ago
•
|
||
[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 | ||
Comment 28•1 years ago
•
|
||
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
Updated•1 years ago
|
Comment 29•1 years ago
|
||
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.
Assignee | ||
Comment 30•1 year ago
•
|
||
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.
Reporter | ||
Comment 31•1 year ago
|
||
hello any updates?
Assignee | ||
Comment 32•1 year ago
|
||
Hi :Hafiizh I am continuing to look into this issue. Unfortunately the initial 'fix' caused a regression.
Updated•1 year ago
|
Comment 33•1 year ago
|
||
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.
Assignee | ||
Comment 34•1 year ago
•
|
||
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'
Comment 35•1 year ago
|
||
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?
Comment 36•1 year ago
|
||
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.
Assignee | ||
Comment 37•1 year ago
|
||
Comment 38•1 year ago
|
||
Assignee | ||
Comment 39•1 year ago
|
||
Assignee | ||
Comment 40•1 year ago
•
|
||
Comment 41•1 year ago
|
||
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).
Assignee | ||
Comment 42•1 year ago
|
||
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
Comment 43•1 year ago
|
||
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
Updated•1 year ago
|
Comment 44•1 year ago
|
||
Comment 45•1 year ago
|
||
Authored by https://github.com/t-p-white
https://github.com/mozilla-mobile/firefox-android/commit/f57ec02d1b234bb5e8ed5fd4ae5795bbde1f3225
[main] Bug 1823316 - Use 'Snackbar' themed Dialog to notify on making app full-screen
Comment 46•1 year ago
|
||
No need to uplift to Beta 120. This fix can ride the trains with 121.
Updated•1 year ago
|
Updated•1 year ago
|
Comment 47•1 year ago
|
||
Reporter | ||
Comment 48•1 year ago
|
||
How is the decision about the bounty? because the sec-bounty flag has not been set
Updated•1 year ago
|
Updated•1 year ago
|
Comment 49•1 year ago
|
||
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.
Comment 50•10 months ago
|
||
Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter
Updated•9 months ago
|
Updated•8 months ago
|
Description
•