Closed Bug 1783561 (CVE-2023-28159) Opened 2 years ago Closed 2 years ago

Block the fullscreen notification on Android using download popups

Categories

(Fenix :: General, defect, P2)

Product:
Fenix
Component:
General
Platform:
Unspecified
Android
Type:
defect

Tracking

(firefox109 wontfix, firefox110 wontfix, firefox111+ fixed)

Status:
VERIFIED FIXED
Milestone:
111 Branch
Tracking Flags:
Tracking Status
firefox109 --- wontfix
firefox110 --- wontfix
firefox111 + fixed

People

(Reporter: haxatron1, Assigned: petru)

References

Details

(Keywords: reporter-external, sec-high, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main111+])

Attachments

(12 files, 6 obsolete files)

poc.html
349 bytes, text/html
Details
poc.mp4
846.88 KB, video/mp4
Details
AlwaysShowTheSnackbar.mp4
5.29 MB, video/mp4
Details
For_1783561_-_AC_Patch1_Stop_BrowserToolbarBehavior_positioning_the_snackbar_.patch
7.51 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
Bug_1783561_-_Fenix_Patch1_Control_the_snackbar_positioning_from_Fenix.patch
22.24 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
Bug_1783561_-_Fenix_Patch2_Show_the_download_dialog_as_an_Android_View.patch
43.97 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
Bug_1783561_-_Fenix_Patch3_Show_the_3rd_party_download_dialog_as_an_Android_View.patch
7.18 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
For_1783561_-_AC_Patch_2_Allow_a_custom_View_for_first_party_downloads.patch
14.71 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
For_1783561_-_AC_Patch3_Allow_a_custom_view_for_3rd_party_downloads.patch
22.24 KB, patch
jonalmeida
: review+
tjr
: sec-approval+
Details | Diff | Splinter Review
[mozilla-mobile/fenix] Bug 1783561 - Allow for better styling and control of the download dialogs (#28572)
50 bytes, text/x-github-pull-request
Details | Review
[mozilla-mobile/firefox-android] Bug 1783561 - Allow clients better styling and control of the download dialogs (#565)
58 bytes, text/x-github-pull-request
Details | Review
advisory.txt
344 bytes, text/plain
Details
Attached file poc.html —

When entering fullscreen, the fullscreen notification is essential to warn users that they are entering fullscreen on the site to prevent spoofing attacks as the fullscreen mode lacks the omnibox. See https://www.mozilla.org/en-US/security/advisories/mfsa2022-17/#CVE-2022-29914 for instance.

It is possible to block the fullscreen notification on Firefox by using download popups.

REPRODUCTION STEPS (Tested on Firefox 103.2.0 on Android) :

  1. Open the file below on Firefox on Android (with the omnibox at the bottom, which seems to be the default for me.)
  2. Click on the button, the download popup will appear below which will block the fullscreen notification which will also appear at the bottom
Flags: sec-bounty?
Attached video poc.mp4 —

See above video demonstrating the spoof

Group: firefox-core-security → mobile-core-security
Component: Security → Security: Android
Product: Firefox → Fenix
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: sec-high

Bug for the Android Experience team

Severity: -- → S2
Priority: -- → P2
OS: Unspecified → Android
Assignee: nobody → petru.lingurar

https://github.com/mozilla-mobile/android-components/pull/12971 and https://github.com/mozilla-mobile/fenix/pull/27454 will ensure that the snackbar is always placed on top of the download dialogs.

Attached file GitHub Pull Request (obsolete) —
Attached file GitHub Pull Request (obsolete) —
Component: Security: Android → General

Patch 1 of the proposed AC changes

The snackbar is placed depending on the toolbar so the relation should be
reversed. The toolbar should not know about and control the snackbar.
There could be other siblings that the snackbar wants to position itself by
and so removing this responsability from BrowserToolbarBehavior will help
better support current and future flows.

Attachment #9301155 - Attachment is obsolete: true
Attachment #9301345 - Attachment is obsolete: true

Patch 1 of the proposed Fenix changes.

Control the snackbar positioning from Fenix.
Previously Android-Components - BrowserToolbarBehavior would be responsible
for positioning the snackbar above the toolbar.
With that responsibility removed we can handle in Fenix positioning the
snackbar depending on the toolbar and many more cases - like positioning it
depending on the download dialogs.

Christian noticed that these patches should've been first posted privately to Bugzilla for review and security approval.
Cleaned the changes from the public repos and posted the patches here.
The general direction taken is to place the snackbar above the download dialogs so that no information is lost.

Patch 2 of the proposed Fenix changes

Show the download dialog as an Android View
Tried to mimic the UX of a modal dialog while using Android Views.
This meant including a scrim that would consume all touches and theming the
navigation bar and status bar.
Avoiding a dialog and a separate window will allow the snackbar to see the
new "dialog" as a sibling in a CoordinatorLayout parent and so be able to
position itself based on the new "dialog".
This patch also added "start_download_dialog_layout" from A-C as it leads to
simpler and less code needed to style the layout - colors / shapes with
everything happening in XML versus calculating the values then setting them
programatically.

Attachment #9303697 - Attachment is obsolete: true

Patch 3 of the proposed Fenix changes.

Show the 3rd party download dialog as an Android View
This uses the same direction as the before patch - inflating a new View that
can then serve as an anchor for the Snackbar.
Here we could use directly the AC layout as it needed no special customization.

Attachment #9303698 - Attachment is obsolete: true

Patch 2 of the proposed AC changes.

Allow a custom View for first party downloads.
This will allow clients easily implement their own UI.

Attachment #9303693 - Attachment is obsolete: true

Patch 3 of the proposed AC changes.

Allow a custom view for 3rd party downloads.
This will allow clients easily implement their own UI.

Attachment #9303694 - Attachment is obsolete: true

Saw there were some issues with the initially exported patches. Updated them now.

Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?] [fxdroid]

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

See Also: → https://mozilla-hub.atlassian.net/browse/FXDROID-8
Whiteboard: [reporter-external] [client-bounty-form] [verif?] [fxdroid] → [reporter-external] [client-bounty-form] [verif?]

I reviewed and tested all the patches locally - they work well - thanks! I tested rotation as well and the snack bar persists above it as well.

Comment on attachment 9303692 [details] [diff] [review]
For_1783561_-AC_Patch1_Stop_BrowserToolbarBehavior_positioning_the_snackbar.patch

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The proposed patches will refactor the snackbar and download dialogs functionality.
    One might infer a connection between these features being refactored together but there is no clear indication of a security issue.
  • 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?: The entire stack of patches must be merged together.
    The result is a big refactoring, quite complex and as such is risky to uplift.
  • How likely is this patch to cause regressions; how much testing does it need?: The end result will change the behavior of the snackbar and download dialogs.
    All scenarios involving them should be tested.
  • Is Android affected?: Yes
Attachment #9303692 - Flags: sec-approval?
Attachment #9303695 - Flags: sec-approval?
Attachment #9304135 - Flags: sec-approval?
Attachment #9304136 - Flags: sec-approval?
Attachment #9304137 - Flags: sec-approval?
Attachment #9304138 - Flags: sec-approval?

Comment on attachment 9303695 [details] [diff] [review]
Bug_1783561_-_Fenix_Patch1_Control_the_snackbar_positioning_from_Fenix.patch

Uhg, I'm sorry I didn't check this before or during the break. This is approved to land (both the functionality parts of patches and test parts of patches) IF you are prepared for this to go into Fenix beta this cycle. If you want this to bake in Nightly for longer than the few days before the beta uplift period then we probably want to wait until next release. You can check with RyanVM about the last date he would uplift patches for Beta this cycle.

Attachment #9303695 - Flags: sec-approval? → sec-approval+
Attachment #9304135 - Flags: sec-approval? → sec-approval+
Attachment #9304136 - Flags: sec-approval? → sec-approval+
Attachment #9304138 - Flags: sec-approval? → sec-approval+
Attachment #9304137 - Flags: sec-approval? → sec-approval+
Attachment #9303692 - Flags: sec-approval? → sec-approval+

Thank you Tom!
I think it's preferable for this to have more time in Nightly, will try landing after the next beta cut.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Flags: sec-bounty? → sec-bounty+

Are we planning to backport this to v110 still?

Group: mobile-core-security → core-security-release
status-firefox109: --- → wontfix
status-firefox110: --- → affected
Flags: needinfo?(petru.lingurar)
Target Milestone: --- → 111 Branch

(In reply to Ryan VanderMeulen [:RyanVM] from comment #27)

Are we planning to backport this to v110 still?

Maybe after a few more days after QA validation and the change being out in the wild.
The changes are pretty significant and there is a risk of something getting broken.

Flags: needinfo?(petru.lingurar)

This is getting late for a beta uplift with risks of breaking functionality, Petru, Tom, does it need to ship in 110 or can it ship in 111?

Flags: needinfo?(tom)
Flags: needinfo?(petru.lingurar)

Yes, it is okay to wait a release.

Flags: needinfo?(tom)
status-firefox110: affected → wontfix
tracking-firefox111: --- → +
Flags: needinfo?(petru.lingurar)

The fix for this was verified by QA in https://bugzilla.mozilla.org/show_bug.cgi?id=1812518.

Status: RESOLVED → VERIFIED
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main111+]
Attached file advisory.txt —
Alias: CVE-2023-28159
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: