1742692 - (CVE-2022-22736) Firefox attempts to load some modules from the install directory if PreferSystem32Images is on

Status: RESOLVED FIXED

(Core :: DLL Services, defect, P3)

Description

[Toshihito Kikuchi [:toshi]]

For Windows 10, we enable PreferSystem32Images in the launcher process, so the main process and the child processes load the system DLLs from the system directory before searching the current directory. However, Firefox still searches the current directory for some system DLLs that are statically dependent on firefox.exe or mozglue.dll. This is not a critical security risk because the install directory is by default protected and a user is responsible for managing it, but this behavior is not intended or ideal.

Here are the list of the target modules.

• version.dll: Firefox.exe has delay-loaded version.dll, but mozglue.dll directly imports version.dll.
• dbghelp.dll: Same as version.dll. This is directly imported from mozglue.dll.
• MSASN1.dll: mozglue.dll statically imports wintrust.dll that delayloads MSASN1.dll.
• CRYPTBASE.DLL: The export table entry of RtlGenRandom is in advapi32.dll, but its destination is in CRYPTBASE.DLL. In this case, even if we load advapi32.dll from the system directory, we search the current directory for CRYPTBASE.DLL.

In Nightly, we also search the current directory for JitPI.dll (Intel VTune profiler?), but the loading code is not compiled in the release version.

Comment 1

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1742692 - Add some modules to mozglue's delayload list. r=mhowell

Comment 2

[Toshihito Kikuchi [:toshi]]

Comment on attachment 9252195 [details]
Bug 1742692 - Add some modules to mozglue's delayload list. r=mhowell

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The security risk that this patch mitigates is DLL planting. It's simple and common, but not easy to achieve as long as Firefox is installed in a protected directory. If Firefox is installed in an unprotected directory, on the other hand, we cannot stop the attack. This is to patch a corner case scenario where attackers cannot replace the existing files but can put new files for some reason. Please note that we cannot protect pre-Win10 platforms with this patch and Chrome has not patched this corner case.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Unknown
• Which older supported branches are affected by this flaw?: all branches
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: Not so hard to backport this, but not sure it's needed.
• How likely is this patch to cause regressions; how much testing does it need?: The regression risk is low. The patch changes DLL dependency and no logics are changed.

Comment 3

[Daniel Veditz [:dveditz]]

Comment on attachment 9252195 [details]
Bug 1742692 - Add some modules to mozglue's delayload list. r=mhowell

As a sec-low bug this no longer needs sec-approval to land.

Comment 4

[Toshihito Kikuchi [:toshi]]

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

Comment on attachment 9252195 [details]
Bug 1742692 - Add some modules to mozglue's delayload list. r=mhowell

As a sec-low bug this no longer needs sec-approval to land.

Thank you for clarifying the process. I'll get landing started.

Comment 5

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

Backed out for causing cppunit test failures:

https://hg.mozilla.org/integration/autoland/rev/f1532eac2190ceaaf552658755762da3bef55abd

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=1ff449af979d75b31a39df54cd394fc2f9bc068e&selectedTaskRun=CuCHwZxvR92veAwrfP4_7Q.0
Failure log: https://treeherder.mozilla.org/logviewer?job_id=359740736&repo=autoland

[task 2021-11-30T20:14:24.739Z] 20:14:24     INFO -  TEST-START | TestMMPolicy.exe
[task 2021-11-30T20:14:25.048Z] 20:14:25     INFO -  mozcrash checking C:\Users\task_163830216965824\AppData\Local\Temp\tmp2mr6d8mm for minidumps...
[task 2021-11-30T20:14:25.049Z] 20:14:25  WARNING -  TEST-UNEXPECTED-FAIL | TestMMPolicy.exe | test failed with return code 3221226505

Comment 6

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

Add some modules to mozglue's delayload list. r=mhowell
https://hg.mozilla.org/integration/autoland/rev/63faa0dca4627f57608b88839757727462a18fcb
https://hg.mozilla.org/mozilla-central/rev/63faa0dca462

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Seems minor enough that we don't need to worry about backporting to ESR, but feel free to nominate if you feel strongly otherwise.

Comment 8

[Frederik Braun [:freddy]]

Attachment: advisory.txt