1546154 - Fix xul.dll dependencies to not load user32 and gdi32 when running in a sandboxed child process with win32k lockdown

Status: RESOLVED FIXED

(Toolkit :: General, task, P3)

Description

[(No longer employed by Mozilla) Aaron Klotz]

(Filing this under toolkit since the moz.build that governs xul.dll's linking is under toolkit/library.)

As we learned in bug 1535704, system DLLs may make assumptions about the availability of win32k by checking for the presence of user32, without checking the state of the win32k lockdown process mitigation policy itself.

In order to avoid future occurrences of this, and also to improve memory usage, we should reorganize xul.dll's dependencies so that these libraries are not loaded when they are unnecessary.

Note that there may be transitive dependencies due to other libraries that are imported by xul.

Comment 2

[Bob Owen (:bobowen)]

Attachment: WIP - Prevent load of user32 and gdi32

The next thing that causes the load is at [1] IIRC.
Chromium already has a fix for this [2].

[1] https://searchfox.org/mozilla-central/rev/1a47a74bd5ba89f2474aa27c40bd478e853f3276/ipc/chromium/src/base/command_line.cc#54
[2] https://source.chromium.org/chromium/chromium/src/+/master:base/command_line.cc;l=472-482;drc=185ceda258c2fe6d2530182b174cec35d3ebfc1f

Comment 3

[Chris Martin [:cmartin]]

I took Bob's patch from Comment 2 and broke it up over Bug 1701757 and Bug 1701778. They should be pretty straightforward to land :)

Comment 4

[Bob Owen (:bobowen)]

Attachment: dontLoadUser32.patch

Here's my latest WIP that I need to break up.
It would be interesting if this fixes what you're seeing.

Comment 5

[Chris Martin [:cmartin]]

Your patch got rid of quite a few DLLs that were loading ole32.dll. With a few more functions patched to api-ms-win-core-com-l1-1-0.dll I'm able to start to see content stuff pop up without an ole32.dll load, so that's awesome.

I've switched to another task for now though (investigating the failures from that Try build that I made with the Faux Lockdown), since I suspect you are also currently patching out a bunch of these calls and I don't want to redo the same work :)

Comment 6

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p1: Call CommandLineToArgvW via API set when possible to prevent shell32 loading. r=handyman!

Comment 7

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p2: Delay load DLLs in xul to prevent user32 from automatically loading. r=glandium!

Depends on D124928

Comment 8

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p3: Explicitly load COM functions from combase.dll to prevent ole32 loading. r=Jamie!

Depends on D124929

Comment 9

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p4: pre-load ole32.dll for GMPs that require it for OPM. r=bryce!

This is required because ole32.dll is now delay loaded by xul.dll.

Depends on D124931

Comment 10

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p5: Remove CoInitializeEx call from TaskController. r=bas!

Threads are implicitly members of the multi-threaded apartment and calls to
CoInitializeEx (and CoUninitializeEx) cause user32 to load.

Depends on D124932

Comment 11

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p6: Remove MSCOMInitThreadPoolListener. r=bryce!

Threads are implicitly members of the multi-threaded apartment and calls to
CoInitializeEx (and CoUninitializeEx) cause user32 to load.

Depends on D124933

Comment 12

[Bob Owen (:bobowen)]

Attachment: Bug 1546154 p7: Don't use the content temp dir when win32k is locked down. r=handyman!

Depends on D124934

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbb0b25450f5
p1: Call CommandLineToArgvW via API set when possible to prevent shell32 loading. r=handyman
https://hg.mozilla.org/integration/autoland/rev/7f50fca0c2cd
p2: Delay load DLLs in xul to prevent user32 from automatically loading. r=glandium
https://hg.mozilla.org/integration/autoland/rev/f3e005f5fedc
p3: Explicitly load COM functions from combase.dll to prevent ole32 loading. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/cfda47fb0a46
p4: pre-load ole32.dll for GMPs that require it for OPM. r=bryce
https://hg.mozilla.org/integration/autoland/rev/cef0aa18a3ab
p5: Remove CoInitializeEx call from TaskController. r=bas
https://hg.mozilla.org/integration/autoland/rev/611812ee62a2
p6: Remove MSCOMInitThreadPoolListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/0bd777eee249
p7: Don't use the content temp dir when win32k is locked down. r=handyman

Comment 14

[Marian-Vasile Laza]

Backed out 7 changesets (Bug 1546154) for causing build bustages on ContentProcess.cpp.
Backout link
Push with failures
Multiple failures: R1, bc2
Failure Log

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b7e29767850
p1: Call CommandLineToArgvW via API set when possible to prevent shell32 loading. r=handyman
https://hg.mozilla.org/integration/autoland/rev/e3b1d57b425d
p2: Delay load DLLs in xul to prevent user32 from automatically loading. r=glandium
https://hg.mozilla.org/integration/autoland/rev/e9ac3dc0abbc
p3: Explicitly load COM functions from combase.dll to prevent ole32 loading. r=Jamie
https://hg.mozilla.org/integration/autoland/rev/216cf15939f4
p4: pre-load ole32.dll for GMPs that require it for OPM. r=bryce
https://hg.mozilla.org/integration/autoland/rev/1595ad239e4e
p5: Remove CoInitializeEx call from TaskController. r=bas
https://hg.mozilla.org/integration/autoland/rev/11d856647620
p6: Remove MSCOMInitThreadPoolListener. r=padenot
https://hg.mozilla.org/integration/autoland/rev/66b784a0294e
p7: Don't use the content temp dir when win32k is locked down. r=handyman

Comment 16

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/5b7e29767850
https://hg.mozilla.org/mozilla-central/rev/e3b1d57b425d
https://hg.mozilla.org/mozilla-central/rev/e9ac3dc0abbc
https://hg.mozilla.org/mozilla-central/rev/216cf15939f4
https://hg.mozilla.org/mozilla-central/rev/1595ad239e4e
https://hg.mozilla.org/mozilla-central/rev/11d856647620
https://hg.mozilla.org/mozilla-central/rev/66b784a0294e