1571182 - Assertions at CoWaitForMultipleHandles

Status: RESOLVED DUPLICATE of bug 1821963

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

Description

[Tom Ritter [:tjr]]

Removing either the WinBuiltinUser sid or the WinWorld sid produces tons of CoWaitForMultipleHandles failures on many tests.

An example is dom/media/test/test_background_video_suspend.html

Comment 1

[Tom Ritter [:tjr]]

hresult is 800706cc == RPC_S_DUPLICATE_ENDPOINT

Which, you know, is not a listed return code from CoWaitForMultipleHandles but why let that stop you.

Comment 2

[Tom Ritter [:tjr]]

I'm gonna have to ask Aaron for advice on what to try/check on this one....

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

This is a leaky abstraction, I'm afraid. COM is clearly trying to do something under the hood that is failing, but I have no idea what that "something" that is failing would be.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

What are the call sites for the affected CoWaitForMultipleHandles? Is this the stuff in ipc/glue?

Comment 5

[Tom Ritter [:tjr]]

(In reply to Aaron Klotz [:aklotz] from comment #4)

What are the call sites for the affected CoWaitForMultipleHandles? Is this the stuff in ipc/glue?

Yup https://searchfox.org/mozilla-central/rev/b3fd653bc6078b3be4a8d06db39eddc5714755da/ipc/glue/WindowsMessageLoop.cpp#913

Comment 6

[Tom Ritter [:tjr]]

I replaced CoWaitForMultipleHandles with WaitForSingleObjectEx and... it (the single test I was using to observe behavior) passed!

It would be unfortunate to have to delay Restricted Token until we have win32k lockdown though, so I'll probably keep digging...

Comment 7

[Bob Owen (:bobowen)]

Do we really need to wait for full win32k or just once we stop using Win32 message pump?
We don't create a window any more but we still do dummy peek messages for native theming.
Once we get rid of native theming we should be able to drop that as well.

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Yeah, I think we can gate that change on the Win32 message pump.

Comment 9

[Tom Ritter [:tjr]]

CoWaitForMultipleHandles is failing calling combase!CheckLrpc which makes sense as a place to fail. Time to go read up on LRPC (Local RPC?) and if these endpoints are securable and I can add access controls to them...

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

They are, and probably use the DACL set here.

Comment 11

[Tom Ritter [:tjr]]

Reading some Chrome code, they allow access to LocalService, which we do not. After adding that, I get the same result (RPC_S_DUPLICATE_ENDPOINT) removing WinWorld; but removing WinBuiltinUser I get a new error REGDB_E_CLASSNOTREG (0x80040154). Still more digging and comparing to do.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Tom Ritter [:tjr] from comment #11)

removing WinBuiltinUser I get a new error REGDB_E_CLASSNOTREG (0x80040154). Still more digging and comparing to do.

Complete speculation on my part, but COM might be trying to query the registry for some info about a CLSID and is being denied access.

Comment 13

[Tom Ritter [:tjr]]

Dug into this (removing WinBuiltinUser) more. As I've moved towards matching Chrome's security permissions on the call to CoInitializeSecurity, it's changed slightly. I'm now failing in combase!wRegOpenInterfaceKey with 0x80040150 (REGDB_E_READREGDB).

The call has the arguments:

5:139> dv
           riid = 0x000001dc`01dc0930 {00000134-0000-0000-C000-000000000046}
      lphkeyIID = 0x00000052`93df8ca8
          szKey = wchar_t [256] "???"
             hr = <value unavailable>
         status = <value unavailable>

Not much to be found on that; but I matched 00000134-0000-0000-C000-000000000046 to IRunDown. So it's trying to open some Registry Key and being denied access.

At this point I've matched Chrome's permissions (I actually have a superset of them - minus the SACL which IIUI is only about logging):

Chrome
	Owner: LocalSystem
	Group: LocalSystem
	DACL: COM_RIGHTS_EXECUTE | COM_RIGHTS_EXECUTE_LOCAL for Local System and Local Service (NT Authority S-1-5-19)
	SACL: Mandatory Label, No Execute Up, ML_MEDIUM

Firefox
	Owner: User
	Group: User's Primary Group
	DACL: COM_RIGHTS_EXECUTE for Local System, Administrators (S-1-5-32-544), User's Sid

My Firefox
	Owner: LocalSystem
	Group: LocalSystem
	DACL: COM_RIGHTS_EXECUTE | COM_RIGHTS_EXECUTE_LOCAL for Local System, Administrators (S-1-5-32-544), User's Sid, LocalService

At one point I searched for 00000134-0000-0000-C000-000000000046 in the registry and never found it; but it is there now. Its permissions include Read access to computername\Users, All Application Packages, an 'Account Unknown' with a large SID that looks like it could be an AppContainer, System, Administrators, and TrustedInstaller.

It makes sense that removing access to the User's SID causes this open to fail; but I'm not sure how to fix it. (I can't edit the permissions in regedit at least.)

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Doesn't seem to be a P1

Comment 15

[Bob Owen (:bobowen)]

I think that this effectively goes away once win32k is enabled, because of bug 1385014.
In fact now that we have native theming we can possible always get rid of this issue, I'll check.

Comment 16

[Bob Owen (:bobowen)]

This possibly wasn't an issue since bug 1385014, but the code was removed in bug 1821963 anyway, so I'm going to duplicate to that bug.