1718796 - Invoke Element.requestFullscreen without Fullscreen Security Notification leads to Address Bar UI spoofing

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, task, P2)

Description

[Irvan Kurniawan (:sourc7)]

Attachment: fullscreen-spoof.bundle.html

After set document.documentElement.hidden to true and load recursive iFrame by setting srcdoc to document.documentElement.innerHTML, then subsequently call Element.requestFullscreen the browser goes into fullscreen without any fullscreen notification ("<domain> is now in fullscreen"), as user unaware page displayed fullscreen it can leads to address bar UI spoofing.

Currently the testcase is works most of the time on Linux (default settings) and Windows (with fullscreen transition disabled), but it still intermittent on transition enabled, even so we can ask the victim to try again (e.g. "You're a robot, please try again") until the spoof is success.

However I have another testcase that works most of the time on Windows even with transition enabled, I'm currently testing this I'll share the testcase once I'm done testing =).

Tested works on:

• Firefox Nightly 91.0a1 (2021-06-30) (64-bit)
• Firefox 89.0.2 (64-bit)
• Firefox 78.11.0esr (64-bit)

Steps to reproduce:

1. Visit attached fullscreen-spoof.bundle.html
2. Hold down "space" or keep clicking until browser goes fullscreen
3. The browser goes into fullscreen without any fullscreen notification

Comment 1

[Irvan Kurniawan (:sourc7)]

Attachment: Firefox - requestFullscreen without Fullscreen Notification.mp4

Comment 2

[Irvan Kurniawan (:sourc7)]

Attachment: Firefox - requestFullscreen without Fullscreen Notification on Windows 10.mp4

(In reply to Irvan Kurniawan (:sourc7) from comment #0)

However I have another testcase that works most of the time on Windows even with transition enabled, I'm currently testing this I'll share the testcase once I'm done testing =).

It turns out the original testcase is works most of the time on Windows 10 using Firefox Nightly on fresh profile (default settings) with Fission enabled at default.

Comment 3

[Irvan Kurniawan (:sourc7)]

Attachment: fullscreen-spooftest1.bundle.html

Hereby the alternative testcase, it works reliably on Fission enabled as well as Fission disabled.

After visit the testcase, hold down "Space" key until the browser goes fullscreen and h2 text "It finally worked!" to appear.

Comment 4

[Irvan Kurniawan (:sourc7)]

Attachment: Firefox - requestFullScreen without Fullscreen Notification on Windows 10 (alternative testcase).mp4

Comment 5

[Daniel Veditz [:dveditz]]

I can confirm the PoC works (fullscreen without the warning message), but I'm not sure how you'd talk people into holding the spacebar down and not have them be suspicious.

Comment 6

[Irvan Kurniawan (:sourc7)]

(In reply to Daniel Veditz [:dveditz] from comment #5)

I can confirm the PoC works (fullscreen without the warning message), but I'm not sure how you'd talk people into holding the spacebar down and not have them be suspicious.

User gesture such as clicking also works as demonstrated on comment 2 with original testcase, I think which user more likely to trust.

Comment 7

[Daniel Veditz [:dveditz]]

I got the testcase from comment 3 to work by double-clicking on it. Slightly harder to elicit than a single click, but still potentially realistic.

Edgar: is fullscreen in your wheelhouse? Any idea why the warning is suppressed here?

Comment 8

[Edgar Chen [:edgar]]

After some investigation, I saw

JavaScript error: resource:///actors/DOMFullscreenChild.jsm, line 34: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIDOMWindowUtils.lastTransactionId]

in the log when the issue happens

So DOMFullscreenChild.jsm stops handing the fullscreen request then we end up doesn't show the warning.
And if I catch the exception in https://searchfox.org/mozilla-central/rev/74abde6afe26cfcaa6ecb3ebefc7a80a781550a7/browser/actors/DOMFullscreenChild.jsm#28, then I could not reproduce anymore.

The next thing that needs to check is why does nsIDOMWindowUtils.lastTransactionId throw NS_ERROR_UNEXPECTED error.

Comment 9

[Edgar Chen [:edgar]]

nsIDOMWindowUtils.lastTransactionId throw NS_ERROR_UNEXPECTED is because we could not get presContext due to we are in the middle of processing navigation. Normally, when navigating to another page, if the previous page has entered into fullscreen mode, we should exit the fullscreen. However, in this case, we are also in the middle of processing the fullscreen request, then the page navigates to another page. I think in this case we should end up exiting the fullscreen because we already navigate away from the page that requests fullscreen.

Comment 10

[Edgar Chen [:edgar]]

Attachment: test.html

Comment 11

[Edgar Chen [:edgar]]

Okay, this is actually nothing related to the reload, but the display: none iframe. I simplify the test a bit, the notification isn't shown if the fullscreen request is made on an iframe that is a descendant of display: none element.

Comment 12

[Edgar Chen [:edgar]]

Attachment: Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

Comment 13

[Edgar Chen [:edgar]]

Comment on attachment 9237704 [details]
Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

Security Approval Request

• How easily could an exploit be constructed based on the patch?: I think it is not trivial to construct an exploit from this patch.
• 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?: None
• How likely is this patch to cause regressions; how much testing does it need?: nsIDOMWindowUtils.lastTransactionId is used only in a few places, and this patch does not change the behavior but uses a more reliable way to get the transaction id of the root PresContext.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 9237704 [details]
Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

sec-approval=dveditz

Thank you for separating the tests out into a separate bug so we can land them later and not give away the game like we would if we landed the test with this security patch. Next time please make the test bug also a hidden bug.

Comment 15

[Daniel Veditz [:dveditz]]

[Tracking Requested - why for this release]:
I know it's late, but this makes for a very convincing and, with echen's testcase, very easy spoof. And unfortunately that simplified testcase has been in public in bugzilla and phabricator for 6 days. Can we get this into the 92 release instead of waiting? The patch looks straightforward.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Is there any way to exploit this on mobile?

Comment 17

[Edgar Chen [:edgar]]

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

Is there any way to exploit this on mobile?

Nope, mobile has a different implementation for fullscreen warning where doesn't have the same issue.

Comment 18

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

Use DocShell to get last transaction id of root PresContext; r=smaug
https://hg.mozilla.org/integration/autoland/rev/bc59a7d754ca876e903acbba2d6721bd2751652f
https://hg.mozilla.org/mozilla-central/rev/bc59a7d754ca

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Please request Release/ESR91/ESR78 approval on this ASAP.

Comment 20

[Edgar Chen [:edgar]]

Comment on attachment 9237704 [details]
Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

Beta/Release Uplift Approval Request

• User impact if declined: When requesting fullscreen on a display: none iframe successfully, there is no fullscreen warning showed, which could lead to address bar UI spoofing.
• 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: 1. Load the test page in comment# 10.

2. Click the request fullscreen button.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Should be low as nsIDOMWindowUtils.lastTransactionId is used only in a few places, and this patch does not change the behavior but uses a more reliable way to get the transaction id of the root PresContext.
• String changes made/needed: None

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug.
• User impact if declined: When requesting fullscreen on a display: none iframe successfully, there is no fullscreen warning showed, which could lead to address bar UI spoofing.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Should be low as nsIDOMWindowUtils.lastTransactionId is used only in a few places, and this patch does not change the behavior but uses a more reliable way to get the transaction id of the root PresContext.
• String or UUID changes made by this patch: None

Comment 21

[Edgar Chen [:edgar]]

Attachment: [ESR78] Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

Comment 22

[Edgar Chen [:edgar]]

Comment on attachment 9238804 [details] [diff] [review]
[ESR78] Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug.
• User impact if declined: When requesting fullscreen on a display: none iframe successfully, there is no fullscreen warning showed, which could lead to address bar UI spoofing.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Should be low as nsIDOMWindowUtils.lastTransactionId is used only in a few places, and this patch does not change the behavior but uses a more reliable way to get the transaction id of the root PresContext.
• String or UUID changes made by this patch: None

Comment 23

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 93.0a1 on Windows 10 x64, macOS 10.15 and on Ubuntu 20.04.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9237704 [details]
Bug 1718796 - Use DocShell to get last transaction id of root PresContext; r=smaug

Approved for 92.0rc2, 91.1esr, and 78.14esr.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/d015edeaf1a7
https://hg.mozilla.org/releases/mozilla-esr78/rev/9d633611fb47

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/16fa80893f36

Comment 27

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 28

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 92.0 on Windows 10 x64, macOS 10.15 and on Ubuntu 20.04.

Comment 29

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 91.1.0esr and on Firefox 78.14.0esr on Windows 10 x64, macOS 10.15 and on Ubuntu 20.04.

Comment 30

[Edgar Chen [:edgar]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #27)

Please visit this google form to reply.

Done.