1696871 - DLL Hijacking in Firefox - malicious version.dll planted in Firefox install directory

Status: RESOLVED WONTFIX

(Core :: Widget: Win32, defect)

Description

[jiechen xia]

Attachment: Dll-hijack.zip

We were informed by CNVD(https://www.cnvd.org.cn/) that they discovered an Dll hijacking vulnerability in Firefox.

Steps to reproduce:
Put the version.dll to "C:\Program Files\Mozilla Firefox" and the malicious code will be executed once FireFox is launched.

Actual results:
Code execution on client side. A cmd.exe was successfully executed once firefox.exe was launched.
This could also lead to privilege escalation if the firefox.exe is run with admin privileges.

Expected results:
As per CNVD's recommendations, the developers should:
Use fully qualified paths for all calls to LoadLibrary, place all the required DLL into the application dir, don't put them into system dir or other dir.

Comment 1

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

This dll gets loaded very early (it's the first CreateFile that fails in the exe's dir, but there others), so my guess is there's nothing we can do to prevent this (shipping all the system DLLs seems infeasible). It doesn't seem like a feasible route for privilege escalation given that running Firefox elevated is already asking for trouble; though the same thing happens with the Maintenance Service (which does run as SYSTEM), that still requires write access into Program Files.

Toshi, is this known and expected? Is there anything we could do to protect against this in the installation directory?

Comment 2

[Toshihito Kikuchi [:toshi]]

Thank you for looping me in. It's a technique known as "Dll planting". Here's an article written by MSRC. If an attacker already has permission to put a file in the install directory, they even replace firefox.exe itself, and we cannot prevent that by our code. I cannot think of any mitigation for this attack from our side.

We have already enabled the mitigation policy PreferSystem32 for the browser process and child processes, so DLL loading in the browser process and child processes is safe, but we cannot enable it for the launcher process, which is a launcher of the browser process.

I agree that this is not the privilege escalation scenario because a malicious process runs with the same privilege as firefox.exe.

Comment 3

[Toshihito Kikuchi [:toshi]]

(In reply to Adam Gashlin (he/him) [:agashlin] from comment #1)

Is there anything we could do to protect against this in the installation directory?

Technically we can monitor a directory using this notification and warn a user if there was an unexpected change, but having a monitoring process sounds overkill to me.

Comment 4

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Do we already have a bug on file this could be duped to? Is there a better component than Installer (which doesn't seem relevant)?

Comment 5

[Toshihito Kikuchi [:toshi]]

I cannot find any other bug claiming the same issue, Dll planting in firefox.exe. I'm not sure what is a right component for this either. Moving this to Win32 because it's Windows specific.

Comment 6

[Daniel Veditz [:dveditz]]

Doesn't Windows have version.dll in it's own system directories? We load things from Firefox install dir, then the windows directories, then after that various path places. The "various path places" are indeed problematic if we haven't found things by then, but it's generally not considered a vulnerability if an attacker has to modify your install or the windows directories -- they can already do worse.

Is CVND claiming they can put the version.dll anywhere else and Firefox will load it?

Comment 7

[jiechen xia]

Attachment: steps.zip

Yes, version.dll in windows system directories, please see the way to reproduce vulnerabilities from attachement.
BTW, CNVD close this vulnerability report after I update the "Dll planting" information on CNVD.

Comment 8

[Daniel Veditz [:dveditz]]

We can't do anything about that. version.dll is never explicitly loaded from Firefox so we can't simply "use the full path" to fix this; it's implicitly loaded by some other library in Windows itself, or looking at the first image probably from the Microsoft redistributable vcruntime140.dll.

We don't try to protect against a malicious actor who can modify the Firefox install directory because they could modify Firefox itself at that point.

Comment 9

[Masatoshi Kimura [:emk]]

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

We can't do anything about that. version.dll is never explicitly loaded from Firefox so we can't simply "use the full path" to fix this; it's implicitly loaded by some other library in Windows itself, or looking at the first image probably from the Microsoft redistributable vcruntime140.dll.

"Real Correct Fix (TM)" explained in bug 1683425 may mitigate that because we will no longer load vcruntime140.dll from our app directory. kernel32.dll is a known dll. We can load all other dlls using delay-load (we will have to do it anyway due to Win32k lockdown.)

We don't try to protect against a malicious actor who can modify the Firefox install directory because they could modify Firefox itself at that point.

I was told that putting a new file might require less privileges than replacing an existing file in bug 1292444.

Comment 10

[Masatoshi Kimura [:emk]]

Although we don't load version.dll explicitly, our launcher process uses version.dll functions here:
https://searchfox.org/mozilla-central/rev/bb37e6fe8bbe383a57a4ad21a201e26416613ca1/browser/app/winlauncher/ErrorHandler.cpp#383,387,392
vcruntime140.dll is not guilty. We should delay-load version.dll with a full path.

Comment 11

[Masatoshi Kimura [:emk]]

(In reply to Toshihito Kikuchi [:toshi] from comment #2)

We have already enabled the mitigation policy PreferSystem32 for the browser process and child processes, so DLL loading in the browser process and child processes is safe

Please don't forget that we still support Windows 7/8.1.

Comment 12

[Masatoshi Kimura [:emk]]

(In reply to Masatoshi Kimura [:emk] from comment #9)

I was told that putting a new file might require less privileges than replacing an existing file in bug 1292444.

For example, if a user extracts a Firefox zip package into %allusersproile%, no other users cannot modify an extracted file unless they have admin privileges. But they can put a new file such as version.dll. I believe this bug is worth fixing.