Closed Bug 1719088 (CVE-2021-29983) Opened 3 years ago Closed 3 years ago

Firefox for Android Lock Exit Fullscreen Mode with Recursive iFrame

Categories

(GeckoView :: General, defect, P1)

Tracking

(firefox90 wontfix, firefox91+ verified, firefox92+ verified)

RESOLVED FIXED
92 Branch
Tracking Status
firefox90 --- wontfix
firefox91 + verified
firefox92 + verified

People

(Reporter: sourc7, Assigned: agi)

References

Details

(Keywords: csectype-spoof, sec-high, Whiteboard: [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?][adv-main91+])

Attachments

(5 files)

When I try to reproduce Bug 1718796 in Firefox for Android, after the browser goes into fullscreen mode I unable to exit the fullscreen using back button, the fullscreen mode is persistent it still persist even navigate to different origin.

Currently the testcase is still intermittent, so there is still chance to able to exit the fullscreen,
probably timing between iFrame element target and entering fullscreen.

Tested on:

  • Firefox Nightly 91.0a1 (Build #2015819979)
  • Firefox Beta 90.0.0-beta.6 (Build #2015818371)
  • Firefox 89.1.1 (Build #2015812947)

Steps to reproduce:

  1. Visit attached fullscreen-persistent.fenix.html
  2. Single tap or double tap on the page
  3. When browser goes into fullscreen
  4. Press back button repeatedly
  5. The browser won't exit the fullscreen, if otherwise after press back try to tap the page again.
Flags: sec-bounty?

On the following video it shows I can't get out of full screen mode even repeatedly pressing the back button, interestingly after focus on the form (which can be done automatically with JavaScript) the Android autocomplete will appear along with the Android status bar displayed in the top bar, which makes the spoof more convincing.

Group: firefox-core-security → mobile-core-security
Type: task → defect
Component: Security → Security: Android
Product: Firefox → Fenix

We can start with sec-high because it could be a very convincing spoof, but it's not quite clear how much of that is due to bug 1718796 and how much due to this specific aspect. (Don't be lulled by the particular POC -- imagine the flashing frames are invisible and the page has something convincing for you to click on)

I'll be trying to fix this for Fenix. Can someone add me on #1718796 ?

Done.

Looking into this I see Fenix & AC working as expected.

The problem with not being able to exit fullscreen is because of the current flow in which the app will call geckoSession.exitFullScreen() which does mEventDispatcher.dispatch("GeckoViewContent:ExitFullScreen", null) and then will wait for a fullscreen update from Gecko here.
onFullScreenChange(false) normally comes when users press back in a fullscreen video but in the case presented here this callback does not fire after we request from GeckoView to exit fullscreen.

Don't know if the app should always exist fullscreen even if the browser is still in fullscreen and not wait for the onFullScreenChange(false) callback. @Christian? @Agi?

Flags: needinfo?(csadilek)
Flags: needinfo?(agi)

In the meantime, looks like there is nothing actionable on Fenix.
If the bug from #1718796 is resolved in Gecko/GeckoView the fix will trickle down to Fenix.

I think we should avoid mismatching the fullscreen state between GV and Fenix, I can definitely see more bugs coming from GV thinking it's in fullscreen and Fenix thinking it's not. I'll try to figure out what's going on here.

Flags: needinfo?(agi)
Flags: needinfo?(csadilek)
Assignee: nobody → agi
Status: NEW → ASSIGNED

The context here is that we have a method called sendToAllChildren which... does not actually do that. It sends events to all ancestors of the browsingContext instead. So when GeckoView is trying to exit fullscreen it will try to send the GeckoView:DOMFullscreenExited message to the root actor and its ancestors (which don't exist) instead of sending it to the children.

Attachment #9232252 - Attachment description: Bug 1719088 - Refactor sendToAllChildren. → Bug 1719088 - Use document.exitFullscreen instead of traversing the tree.

copying from slack:

thinking about this more (and looking at Desktop's code) I think we can do something even simpler
there's a method on Document to exit fullscreen that desktop uses
we need to rename the method too, but we can do it in another patch to avoid pointing fingers at the problem

Comment on attachment 9232252 [details]
Bug 1719088 - Use document.exitFullscreen instead of traversing the tree.

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The patch is worded as a simplification and it doesn't directly point a the fact that fullscreen in iframes is broken. One people start looking at fullscreen it might not take long until they find the problem though.
  • 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?: Yes
  • If not, how different, hard to create, and risky will they be?: Patch should be identical for all branches
  • How likely is this patch to cause regressions; how much testing does it need?: It could break fullscreen in edge cases.
Attachment #9232252 - Flags: sec-approval?
Severity: -- → S3
Component: Security: Android → General
Priority: -- → P1
Product: Fenix → GeckoView

Comment on attachment 9232252 [details]
Bug 1719088 - Use document.exitFullscreen instead of traversing the tree.

Approved to land. I don't think Fenix has an uplift need?

Attachment #9232252 - Flags: sec-approval? → sec-approval+
Group: mobile-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
Flags: sec-bounty? → sec-bounty+
See Also: → 1718796
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?]

Comment on attachment 9232252 [details]
Bug 1719088 - Use document.exitFullscreen instead of traversing the tree.

Beta/Release Uplift Approval Request

  • User impact if declined: A malicious code could lock fullscren in Fenix
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: - Open testcase in bug
  • Verify that full-screen can be exited by pressing "back" on the device
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): simple change, worst case scenario we add some unrelated fullscreen locks
  • String changes made/needed:
Attachment #9232252 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9232252 [details]
Bug 1719088 - Use document.exitFullscreen instead of traversing the tree.

Approved for uplift on the beta branch before Monday's beta to release merge so if can make it into Fenix 91, thanks.

Attachment #9232252 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I've tested it using the steps from the description and I managed to reproduce it with the latest Nightly 92.0a1 (2021-08-02T05:08:16.189754) build.
I also checked on the latest GV where it is also reproducible.
Device used: Google Pixel 4 (Android 11).
Was not reproducible on the latest Nightly using an Android 12 device.

Flags: needinfo?(agi)
Flags: qe-verify+

Weird, I can't reproduce on my Pixel 3a and my S10 on Android 11 (and I could before the change). I'll try other devices too.

Status: RESOLVED → REOPENED
Flags: needinfo?(agi)
Resolution: FIXED → ---

I can't reproduce this anymore on Mi 9T Android 10 (real devices) and Pixel 4 API 30 Android 11 x86_64 (Android Emulator) with Firefox Nightly 92.0a1 (Build #2015825739 arm64) (Build #2015825743 x86_64).

I also tried this on AWS Device Farm (which using real devices) with Google Pixel 4A on Android 11, I can exit the fullscreen everytime I press the back button on Firefox Nightly 92.0a1 (Build #2015825739). On Firefox 90.1.3 (Build #2015824995) pressing back button won't exit the fullscreen.

Attached video 20210803_095101.mp4

I retested it and managed to activate the back button.
Aside from the latest Nightly, I also tested on the latest Beta (91.0.0-beta.6) where it also appears fixed.
I will close the issue as verified.

Flags: qe-verify+
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Flags: qe-verify+
Whiteboard: [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?] → [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?][adv-main90+]
Whiteboard: [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?][adv-main90+] → [keep hidden while bug 1718796 is][reporter-external] [client-bounty-form] [verif?][adv-main91+]
Attached file advisory.txt
Alias: CVE-2021-29983
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: