1394370 - Sandbox security level 3 makes GetClipBox WinAPI function to return NULLREGION for desktop

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P2)

Description

[Denis]

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20170821193225

Steps to reproduce:

I have a Windows desktop application which checks desktop visibility using GetClipBox function:
	HDC desktop_dc = ::GetDC(NULL);
	RECT clip_rect;
	int complexity = ::GetClipBox(desktop_dc, &clip_rect);
	::ReleaseDC(NULL, desktop_dc);
	if (complexity == NULLREGION)
		OutputDebugString(L"   NULLREGION\n");


Actual results:

When Firefox 56 is running GetClipBox sometimes returns NULLREGION like when desktop is invisible. When I set 2 instead of 3 for security.sandbox.content.level then the issue does not occur anymore.


Expected results:

I did not see such issue before for interactive user session. Only for minimized RDP session.

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

Does this affect firefox itself, or some other application that uses Gecko?

Comment 2

[Denis]

No, it affects my own application (which does not uses Gecko). When my application is used along with Firefox 56 it does not work as expected - it "thinks" that desktop is invisible and reports error about this.

Comment 3

[Denis]

I think this is caused by some code in Firefox content process which is dealing with window clipping (or something like this). It works fine using security level 2 (current Firefox release) but do something wrong because of security level 3 in Firefox 56 which restricts access to windows from another processes (as I found out from security/sandbox/chromium/sandbox/win/src/job.cc).

Comment 4

[Bob Owen (:bobowen)]

(In reply to Denis from comment #2)
> No, it affects my own application (which does not uses Gecko). When my
> application is used along with Firefox 56 it does not work as expected - it
> "thinks" that desktop is invisible and reports error about this.

Hi Denis, this looks like an issue in your application not Firefox then.

It sounds like you only need to do your check on the main/parent Firefox process, not on any of the child processes.

I imagine that you would get similar problems with the child processes for Chrome/Chromium and possibly Edge as well.

Comment 5

[Denis]

Hi Bob, the thing is that my application does not interact with Firefox directly. It is a separate application, which regularly checks Windows desktop visibility using the code above (in issue description). And it worked just fine until I ran Firefox 56 beta at the same user session. Starting from this my application started to report that desktop is invisible. So I suppose Firefox is doing something wrong with desktop DC, but I am not sure what it is exactly.

Comment 6

[Denis]

My code worked for several years without this issue. Chrome and Edge does not trigger it.

Comment 7

[Denis]

I can understand why you resolved the issue as INVALID, but I think that it indicates that you have some hidden issues in Firefox caused by such restrictive security level. If you don't have time to investigate this please point me to some Firefox code that can be related to such behavior and I will try to debug it.

Comment 8

[Bob Owen (:bobowen)]

(In reply to Denis from comment #7)
> I can understand why you resolved the issue as INVALID, but I think that it
> indicates that you have some hidden issues in Firefox caused by such
> restrictive security level. If you don't have time to investigate this
> please point me to some Firefox code that can be related to such behavior
> and I will try to debug it.

Ah right, I misinterpreted what you were doing, I thought you were injecting code into other processes.

However, I'm unable to reproduce the problem you are seeing.
That code is always returning COMPLEXREGION for me.

It certainly seems strange that the permissions on our content processes would be affecting the desktop DC returned in a completely separate process.
If you can find reliable steps to reproduce then I could look into it.

Comment 9

[Denis]

First I run my test application that is executing the code with GetClipBox twice a second. Then I am launching Firefox 56 and my application starts to show "NULLREGION". It seems that issue occurs when new tab is finished loading and stops when some popup is shown in Firefox (and sometimes after some other actions).

Comment 10

[Denis]

Attachment: Source code of console app that shows GetClipBox result for desktop

I attached archive with sample console application, which shows GetClipBox result twice a second.
Also I reproduced this issue on Windows 7 as well as on Windows 10 ver.1703.

Comment 11

[Bob Owen (:bobowen)]

(In reply to Denis from comment #9)
> First I run my test application that is executing the code with GetClipBox
> twice a second. Then I am launching Firefox 56 and my application starts to
> show "NULLREGION". It seems that issue occurs when new tab is finished
> loading and stops when some popup is shown in Firefox (and sometimes after
> some other actions).

Thanks for the extra detail, I've managed to reproduce this, although not on a local build yet.
Once I can do that, I can experiment with the sandbox settings, it seems to go away when I try some stronger settings on Beta and Nightly.
This would tally with the fact that Chrome doesn't cause the same problem.

Comment 12

[Bob Owen (:bobowen)]

I've managed to reproduce locally.

It seems this will be fixed when we start using an alternate desktop (bug 1229829), which should be landing fairly soon.

Comment 13

[Denis]

Good to know that you are reproducing it too. Do you have any idea why this happens? Maybe I can do/trigger something in my application to overcome this issue? I mean maybe when I receive NULLREGION I can do something to revert desktop DC state to return COMPLEXREGION?

Comment 14

[Bob Owen (:bobowen)]

Looks like it's the use of JOB_OBJECT_UILIMIT_HANDLES UI Restriction that triggers this.

My guess is that an attempted operation in the child fails and causes it, although the fact that it affects the Region returned in a different process almost seems like a Windows bug.
I haven't managed to get anywhere with working out what code triggers it when we are using JOB_OBJECT_UILIMIT_HANDLES.

Using a separate desktop (bug 1229829) presumably isolates our content process further to prevent this side-effect.

Comment 15

[Denis]

Hi Bob, do you know will sandbox level 3 be turned on in Firefox 56 release by default?

Comment 16

[Bob Owen (:bobowen)]

(In reply to Denis from comment #15)
> Hi Bob, do you know will sandbox level 3 be turned on in Firefox 56 release
> by default?

Yes it will, but hopefully your issue should only last a release, as we should be turning on Alternate Desktop in 57.

Comment 17

[Denis]

Thank you, Bob. I checked 57 alpha and it seems it does not have this issue.

Comment 18

[Bob Owen (:bobowen)]

(In reply to Denis from comment #17)
> Thank you, Bob. I checked 57 alpha and it seems it does not have this issue.

Thanks for confirming.
I'll mark this as resolved, but Fx56 as affected still.

Comment 19

[Gerry Chang [:gchang]]

Too late for 56. Mass won't fix for 56.

Comment 20

[Bob Owen (:bobowen)]

Unfortunately we had to back the alternate desktop out of 57, so for the moment this issue will still be there.

I can reproduce the other issue (bug 1400637) that alternate desktop and an anti-virus cause in combination, so if I can find a solution there is a chance that we can still get this turned on again in 57.
However, given the importance of the 57 release I suspect we'll be more cautious about any uplifts.

Assuming you don't use Webroot SecureAnywhere (the AV causing problems in bug 1400637), then you could change security.sandbox.content.level to 4 in about:config to get the alternate desktop.

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Bob, what is the plan wrt to 57? Not super clear to me? Wontfix again?

Comment 22

[Bob Owen (:bobowen)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #21)
> Bob, what is the plan wrt to 57? Not super clear to me? Wontfix again?

We've decided the alternate desktop is too risky for 57, because of AV issues.