1656741 - (CVE-2020-26953) Entering fullscreen without UI message showing

Status: VERIFIED FIXED

(Firefox :: Security, defect, P3)

Description

[Abdulrahman Alqabandi]

Attachment: letsrepro.html

OS/Firefox version:
Works on Firefox 79.0 (64-bit) using Windows 10 Pro
Works on Nightly 81.0a1 (2020-08-01) (64-bit) using Windows 10 Pro
Works on Firefox 79.0 (64-bit) using macOS Catalina Version 10.15.5

Does not work on Firefox 79.0 (64-bit) using Ubuntu 18.04.4 desktop

Repro:

1. Host attached PoC HTML in an HTTP server (localhost will do, if other then make sure its HTTPS)
2. Open the hosted 'letsrepro.html' and click the anchor tag

Result:
Firefox enters full screen mode without the security UI saying you are entering fullscreen appearing.

Expected:
When entering full screen mode, the UI indicating so should always appear to prevent security concerns.
See Bug 1432856

Other info:
For some reason a WebSocket connection attempt has to happen first for this to work
If I remove 'new WebSocket("ws://nonexistent");' from the PoC, it does not work.

Comment 1

[:Gijs (he/him)]

Al, can you take a look at what's going on here? There's some timing involved, I think (I can repro on Windows but not on mac, where we just immediately leave fullscreen again), but not showing the warning is bad.

Comment 2

[Alphan Chen [:alchen]]

Sure, I will update what I find.

Comment 3

[Alphan Chen [:alchen]]

(In reply to :Gijs (he/him) from comment #1)

Al, can you take a look at what's going on here? There's some timing involved, I think (I can repro on Windows but not on mac, where we just immediately leave fullscreen again), but not showing the warning is bad.

I also see the symptom of leaving fullscreen immediately on mac and linux.
I can enter the fullscreen and see the warning by clicking the page again.

If I modified the timeout from 260 to 1260, it works fine.
Besides, I don't see any difference between the file with or without 'new WebSocket("ws://nonexistent");'.

-			setTimeout(x=>{location='?go#go';}, 260);
+			setTimeout(x=>{location='?go#go';},1260);

Hi reporter,
could you try to extend the second timeout and update the result?

Comment 4

[Abdulrahman Alqabandi]

Hello triager,

For me the websocket is needed AFAICT. Every time I remove it I stop reproducing. Applying your changes did not result in repro for MacOS.

On MacOS the original repro stopped working but then I modified two things:

			
			
			setTimeout(()=>{document.body.requestFullscreen({})},106);
			setTimeout(x=>{location='?gfo#go';},200);//<---- lowered the timeout and changed the URL a bit to prevent cache

With the above modifications it started working on MacOS and Windows 10 just fine.

My guess is that once you run it, subsequent runs will ruin the timing due to cache. Will work on a more reliable PoC and update if I get one with 100% on all OS. The WebSocket request is probably just adding to the timing aspect, not sure why it is required for me.

Comment 5

[Abdulrahman Alqabandi]

I misused the markdown and part of the code above did not show.

		new WebSocket("ws://facebook.com");//<----- changed the websocket to an existing domain
			
			
			setTimeout(()=>{document.body.requestFullscreen({})},106);
			setTimeout(x=>{location='?gfo#go';},200);//<---- lowered the timeout and changed the URL a bit to prevent cache

Comment 6

[:Gijs (he/him)]

Al, can you try again based on qab's comments?

Comment 7

[Alphan Chen [:alchen]]

(In reply to Abdulrahman Alqabandi from comment #0)

Repro:

1. Host attached PoC HTML in an HTTP server (localhost will do, if other then make sure its HTTPS)

In the first trial, I test it on a localhost. (http://localhost:8000)
The result is in comment 3.

If I modified the file based on comment 4 and comment 5, I still saw the same symptom.
The symptom is leaving fullscreen immediately on mac and linux.

• changed the websocket to an existing domain
• lowered the timeout (setTimeout(x=>{location='?gfo#go';},200);)

This time I also tried to use "https://localhost:4443" to do the test on mac and linux.
In this setup, after clicking the anchor tag, FF won't enter fullscreen with "new WebSocket("ws://facebook.com")" or "new WebSocket("ws://nonexistent");".
If I removed "new WebSocket(...)", the behavior is leaving fullscreen immediately.
I also tried it on Chome. Chome has the same behavior as FF.

:qab, could you also update the behavior of Chome in your setup?
If possible, could you provide the video which can show the problem on your side?

For me, the problem is still unclear.

1. Is the problem related to "new WebSocket()"?
2. Is the problem related to "change the URL right after entering fullscreen"?

:Gijs, is it possible to ask QA to reproduce the symptom?
We need a solid STR to investigate the problem.

Comment 8

[:Gijs (he/him)]

(In reply to Alphan Chen [:alchen] from comment #7)

:Gijs, is it possible to ask QA to reproduce the symptom?

I don't think QA have the skills to adapt the testcase to make it 100% reproducible. If they reproduce and you cannot, what's the point?

We need a solid STR to investigate the problem.

The issue reproduced for me on Windows. (see comment #1)

Do you not have access to a Windows machine at all?

Comment 9

[Alphan Chen [:alchen]]

(In reply to :Gijs (he/him) from comment #8)

The issue reproduced for me on Windows. (see comment #1)

Do you not have access to a Windows machine at all?

I will try to find a windows.
If you can reproduce the problem on windows, could you help to classify these two questions?

Is the problem related to "new WebSocket()"?
Is the problem related to "change the URL right after entering fullscreen"?

Comment 10

[Alphan Chen [:alchen]]

(In reply to Alphan Chen [:alchen] from comment #7)

:qab, could you also update the behavior of Chome in your setup?
If possible, could you provide the video which can show the problem on your side?

For me, the problem is still unclear.

1. Is the problem related to "new WebSocket()"?
2. Is the problem related to "change the URL right after entering fullscreen"?

Comment 11

[:Gijs (he/him)]

In case it makes a difference, I reproduced on Windows by saving the testcase to disk and opening from the file using file:///. Of course this means that subsequent requests (on a fast SSD) to the relative ?go#go link are almost instantaneous.

(In reply to Alphan Chen [:alchen] from comment #9)

Is the problem related to "new WebSocket()"?

Without the websocket connection it doesn't reproduce.

Is the problem related to "change the URL right after entering fullscreen"?

Without changing the URL, the warning comes up fine. Also, if you change the relative link to only point to #go, the warning comes up fine.

Do we check for the URL/document that requested fullscreen and that actually enters fullscreen somewhere?

I wonder if the websocket connection breaks bfcache, and that causes us to create a different windowglobal, and that probably affects the fullscreen actor. Nika, do you know whether websockets break bfcache and whether this is plausible?

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Off the top of my head, I think that an active WebSocket connection would disable bfcache, yeah. You can check if this is caused by that change in behaviour by removing the websocket connection, and instead adding an "unload" event listener on the document, which should also suppress bfcache.

Comment 13

[:Gijs (he/him)]

Using window.addEventListener("unload", console.log) instead of the web socket seems to work, too (ie causes the warning not to come up while the window still goes into fullscreen).

Comment 14

[Abdulrahman Alqabandi]

Attachment: 1.html

(In reply to Alphan Chen [:alchen] from comment #10)

:qab, could you also update the behavior of Chome in your setup?
Does not repro on Chrome AFAICT.

If possible, could you provide the video which can show the problem on your side?
Certainly, attached in the next comment.

For me, the problem is still unclear.

1. Is the problem related to "new WebSocket()"?
   From the subsequent comments, looks like its an issue with bfcache more than WebSocket itself.

2. Is the problem related to "change the URL right after entering fullscreen"?
   Yes.

In the console it says "TypeError: Fullscreen request aborted" when the PoC reproduces successfully. Firefox is definitely getting confused due to the navigation.

I was able to come up with a new PoC which seems to be reliable on all platforms (Windows, Linux and MacOS), at least for me.

Comment 15

[Abdulrahman Alqabandi]

Attachment: poc.mp4

Oops the previous comment mixed my answers with the quotes (keep in mind)

This is the video PoC showcasing it working in three different set ups.

Comment 16

[:Gijs (he/him)]

I can reproduce on macOS where I couldn't before using the new PoC. Alphan, can you take a look again? Thank you.

Comment 17

[Alphan Chen [:alchen]]

By the new Poc, I can reproduce the problem.
Actually, the symptom is FF don't leave the fullscreen normally.
At that time, "aActor.requestOrigin" is undefined.
https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/browser/base/content/browser-fullScreenAndPointerLock.js#573
https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/browser/actors/DOMFullscreenParent.jsm#163

From my experiments, the timing is a big factor.
I can reproduce the problem easily when the timeout is less than 15.

setTimeout(x=>{location=window.URL.createObjectURL(new Blob([blobContent], { type: "text/html" }));},1);

Comment 18

[Alphan Chen [:alchen]]

The fix works on my Ubuntu.
I will verify the result on other platforms when the build is ready.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5775c0faf0a7e2c0384250e0881dd8d1dec8a8a

Comment 19

[:Gijs (he/him)]

It looks like Alphen was affected by the layoffs. :-(

I looked at this, but the patch that Alphen pushed to try didn't fix the issue for me.

From local logging, it appears the parent process actor gets a DOMFullscreen:Request message, and then we unload the document, and the next thing the parent actor hears is that the actor is being destroyed. Then the parent gets a MozDOMFullscreen:Entered event.

The warning is normally generated by the MozDOMFullscreen:NewOrigin event (forwarded to the parent by the child), which is never seen by either of the bits of frontend code involved (again, based on logging). It looks like it is dispatched asynchronously at https://searchfox.org/mozilla-central/rev/19c23d725f27d0989e4a60f36d64004cebb39736/dom/base/Document.cpp#13444-13449 . I suspect that it never fires or is never seen by frontend because presumably the original document is destroyed while the event is queued, and/or we decide we don't need to re-emit it when the document to which we navigate is same-origin.

Olli, does this seem like an accurate diagnosis (if you're not the right person, please forward the needinfo to whoever still knows about our fullscreen code...)? I'm pretty confused, because the code in the PoC invokes document.body.requestFullscreen and so I'd have expected that the document being unloaded would mean we would interrupt our entering into fullscreen and abort it using the relevant APIs from the child to the parent, given that the element in question is no longer going to be around... :-\

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

What I don't understand is that why frontend doesn't show the normal warning (nothing to do with MozDOMFullscreen:NewOrigin).

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

But anyhow, can fix this.

Comment 22

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug] from comment #20)

What I don't understand is that why frontend doesn't show the normal warning (nothing to do with MozDOMFullscreen:NewOrigin).

Can you elaborate on what you mean? The only warning I'm aware of is keyed off this event, it seems.

We could change what we use to show the warning, if that would help?

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1656741, inform parent process about canceled fullscreen request

Comment 24

[Olli Pettay [:smaug][[e-mail]]]

The wip patch isn't necessary quite right for nested iframes.

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9175268 [details]
Bug 1656741, inform parent process about canceled fullscreen request

Or perhaps this should be fixed in a bit different way, since I think, this was working while framescripts were used.

Comment 26

[:Gijs (he/him)]

Thanks, Olli! Let me know if I can help in any way.

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

This should be fixed in a difference way. JS actor stuff gets confused here.

Comment 28

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Bug 1656741

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

Steven has been looking into other fullscreen related issues recently and said he could help here. (thanks!)

https://bugzilla.mozilla.org/attachment.cgi?id=9177090 helps with the issue here (testing with https://bugzilla.mozilla.org/attachment.cgi?id=9169053)
but I think there is another problematic case if parent gets "DOMFullscreen:Request" and then didDestroy is called, since at that point
this._fullscreenWindow is null, I think, and also window.document.documentElement.hasAttribute("inDOMFullscreen") returns false.

Comment 30

[Steven MacLeod [:smacleod]]

Attachment: Bug 1656741, r=Gijs!

Comment 31

[Steven MacLeod [:smacleod]]

Attachment https://bugzilla.mozilla.org/attachment.cgi?id=9178482 should fix things as far as I can tell. Alphan's original patch that worked on linux went part of the way, but based on timing (which I found changed drastically between mac and linux) it wouldn't catch all cases. This patch does a lot more checking for actor's being destroyed before attempting to send a msg to transition the child to Fullscreen. I also needed to keep track in the parent when the msg had already been sent so that if the child died before it could reply we would still cleanup.

I have yet to test this on Windows, but I'm decently confident it will fix things based on my tracing/logging/testing of the pieces involved here.

Comment 32

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

https://hg.mozilla.org/integration/autoland/rev/3e884c48633fb4017402ef2c7053f8d947676dd5
https://hg.mozilla.org/mozilla-central/rev/3e884c48633f

Comment 33

[Hani Yacoub, Desktop QA]

Attachment: ice_video_20201020-105723.webm

Attaching the results of testing on Firefox Nightly 81.0a1 (2020-08-01) (on the left) and on the latest Firefox Nighty (On the right).
I'm not sure if exiting the full screen mode is the expected behavior here or should I look for something else here?
Thanks.

Comment 34

[Steven MacLeod [:smacleod]]

(In reply to Hani Yacoub from comment #33)

Attaching the results of testing on Firefox Nightly 81.0a1 (2020-08-01) (on the left) and on the latest Firefox Nighty (On the right).
I'm not sure if exiting the full screen mode is the expected behavior here or should I look for something else here?
Thanks.

Yup, exiting full screen mode is the expected behaviour with the fix. Looks like it is working as intended.

Comment 35

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox Nightly 84.0a1 (2020-10-20) on Windows 10 x64, MacOS 10.14 and on Ubuntu 20.04 x64.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to ESR78 or is riding mainline to release sufficient?

Comment 37

[Steven MacLeod [:smacleod]]

[Tracking Requested - why for this release]: sec-moderate bug with a relatively low risk patch.

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

Is this something we should consider uplifting to ESR78 or is riding mainline to release sufficient?

Ya, probably worth considering. Patch coming soon, just need to make sure everything is still working properly after that graft.

Comment 38

[Steven MacLeod [:smacleod]]

Attachment: Bug 1656741, r=Gijs

Comment 39

[Steven MacLeod [:smacleod]]

Comment on attachment 9185496 [details]
Bug 1656741, r=Gijs

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a low risk patch fixing a sec-moderate bug.
• User impact if declined: A malicious website may full-screen itself without showing a warning that the page has entered full-screen.
• Fix Landed on Version: 83
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This fix was landed to central ~1 month ago and does not appear to have caused problems there or in the 83 beta, making it relatively low risk IMO.
• String or UUID changes made by this patch:

Comment 40

[Pascal Chevrel:pascalc]

Comment on attachment 9185496 [details]
Bug 1656741, r=Gijs

Approved for ESR, thanks.

Comment 41

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 83.0b8 on Windows 10 x64, MacOS 10.14 and on Ubuntu 20.04 x64.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/b8536fb3f531

Comment 43

[Hani Yacoub, Desktop QA]

Verified as fixed on Firefox 78.4.1esr on Windows 10 x64, MacOS 10.14 and on Ubuntu 20.04 x64.

Comment 44

[Tom Ritter [:tjr]]

Attachment: advisory.txt