1361326 - (CVE-2017-7755) DLL Hijacking Firefox installer

Status: VERIFIED FIXED

(Firefox :: Installer, defect)

Description

[Yuji Tounai]

Attachment: dll.zip

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.81 Safari/537.36

Steps to reproduce:

1.Create a malicious BCRYPT.dll file and save it in your "Downloads" directory.
2.Download 'Firefox Setup Stub 53.0.exe' and save it in your "Downloads" directory.
3.Execute 'Firefox Setup Stub 53.0.exe' from your "Downloads" directory.
4.Malicious dll file gets executed.



Actual results:

trojan DLL loads cmd.exe and alert the dialog 


Expected results:

DLL file on a Windows computer is placed in the default downloads directory with the Firefox installer, the Firefox installer will load this DLL when it is launched. In circumstances where the installer is run by an administrator privileged account, this allows for the downloaded DLL file to be run with administrator privileges. This can lead to arbitrary code execution from a privileged account.

Comment 1

[Molly Howell (she/her) (mostly inactive)]

Thanks for the report. I'm not able to reproduce this using the provided DLL, because we don't load bcrypt.dll in the installer. Normally when we get DLL hijacking issues for DLL's we don't load, it's due to another application injecting them into every process. Since bcrypt.dll is a Windows file, that explanation is less likely, but still possible (a DLL being injected could be loading bcrypt.dll). Reporter, would you mind checking for any applications you might have doing something like that?

Additionally, this only constitutes a privilege elevation if UAC is disabled or can be bypassed, because the downloaded .exe does not try to elevate; it extracts a different one into a temp folder and that one is the one that elevates, so that happens in a separate process.

Comment 2

[Yuji Tounai]

(In reply to Matt Howell [:mhowell] from comment #1)
> Thanks for the report. I'm not able to reproduce this using the provided
> DLL, because we don't load bcrypt.dll in the installer. Normally when we get
> DLL hijacking issues for DLL's we don't load, it's due to another
> application injecting them into every process. Since bcrypt.dll is a Windows
> file, that explanation is less likely, but still possible (a DLL being
> injected could be loading bcrypt.dll). Reporter, would you mind checking for
> any applications you might have doing something like that?
> 
> Additionally, this only constitutes a privilege elevation if UAC is disabled
> or can be bypassed, because the downloaded .exe does not try to elevate; it
> extracts a different one into a temp folder and that one is the one that
> elevates, so that happens in a separate process.

Hello,
I came across this. I also reported that the alert went up when I tried another Windows 10 PC. So I have not examined other dll yet.
I checked it with "Dependency Walker" on my PC, but BCPYCT.DLL has been loaded in several places.
How can I send dwi file?

Comment 3

[Yuji Tounai]

According to "Dependency Walker" in my environment the following dll is also read from the directory where the installer was placed. It is unconfirmed whether exploit is possible

ADVPACK.DLL
APPHELP.DLL
CRYPTSP.DLL
DBGHELP.DLL
DWRITE.DLL
MPR.DLL
MSIMG32.DLL
NETUTILS.DLL
POLICYMANAGER.DLL
SSPICLI.DLL
WINSTA.DLL

Comment 4

[Molly Howell (she/her) (mostly inactive)]

Okay, I do see BCRYPT.DLL in Dependency Walker. Looks like it gets pulled in ADVAPI32.DLL. But I don't see it actually get loaded at runtime, which is why the exploit doesn't work. Something is different between our Windows 10 systems that's causing this behavior, and I'm not immediately sure what it could be.

Comment 5

[Yuji Tounai]

Hello
I also tried dlls other than BCRYPT.DLL but I could not run it. 
And on my Windows 8.1 PC, it was not possible to run using BCRYPT.DLL.

However, in my Windows 8.1 PC it was possible to inject with BCRYPTPRIMITIVES.DLL.

In the case of Windows 8.1, it is easy to understand because an error dialog will appear if you place an empty BCRYPTPRIMITIVES.DLL in the same folder as Firefox Setup Stub 53.0.exe (this may be your own environment as well).

Also, other applications like PuTTY seem to have DLL hijacking vulnerability by BCRYPT.DLL.
Http://www.chiark.greenend.org.uk/~sgtatham/putty/wishlist/vuln-indirect-dll-hijack-2.html

I hope this will help solve the problem.
Regards,

Comment 6

[Molly Howell (she/her) (mostly inactive)]

Attachment: test installer

I'm attaching a modified installer that I've built. Would you mind checking to see if this installer is still vulnerable? On my machine, it does not attempt to load any modules from anywhere but the system directory, but since we know yours is behaving a bit differently I'd like to get confirmation before I start implementing that fix for real. Thanks.

Comment 7

[Yuji Tounai]

I tried the modified installer on my PC. BCRYPT.DLL/BCRYPTPRIMITIVES.DLL was not loaded.
Thank you.

Comment 8

[Molly Howell (she/her) (mostly inactive)]

Okay, great! Thank you for checking.

Comment 9

[Yuji Tounai]

I found that the crashreporter.exe also had a vulnerability to load bcrypt.dll as well as the reported vulnerability.

Since it is difficult to put dll in the same directory as crashreporter.exe, I think it is very difficult to attack in practice, but a similar report in the past have been reported(https://bugzilla.mozilla.org/show_bug.cgi?id=945192) so I report.

Comment 10

[Molly Howell (she/her) (mostly inactive)]

Attachment: Patch

This patch makes two changes:
1) Alters the linker settings the 7-zip SFX stub is built with so that only strictly required libraries are linked and all possible DLL's are delay-loaded. kernel32 and msvcrt cannot be delay-loaded, but they are not subject to this vulnerability.
2) Invokes SetDefaultDllDirectories() early at runtime, before any delay-loading occurs, to ensure that DLL's are only loaded from system32 directory, since the SFX stub uses only DLL's which are found there. This applies to our directly required DLL's and also to their dependencies, which are the actual sources of the vulnerabilities here.

The patch also contains a binary rebuilt with those changes, since this binary is kept in-tree rather than compiled at application build-time.

Comment 11

[Molly Howell (she/her) (mostly inactive)]

(In reply to yujitounai from comment #9)
> I found that the crashreporter.exe also had a vulnerability to load
> bcrypt.dll as well as the reported vulnerability.
> 
> Since it is difficult to put dll in the same directory as crashreporter.exe,
> I think it is very difficult to attack in practice, but a similar report in
> the past have been
> reported(https://bugzilla.mozilla.org/show_bug.cgi?id=945192) so I report.

Thanks for pointing this out; to save you the effort, I've filed bug 1362596 for getting the crash reporter fixed and indicated there that credit for finding it belongs to you.

Comment 12

[Robert Strong (they/them - no direct email)]

Comment on attachment 8865037 [details] [diff] [review]
Patch

Looks good from a code perspective. You might consider using decltype as was done here
bug 945192 comment #284

I assume this works with Win2K and above so we can provide unsupported messages. If not then you should run this by product to get their buy in.

Kamil has provided testing for these types of changes in the past as you can also see in bug 945192 and in other bugs. It would be a good thing to ask him if he could verify this one as well.

Comment 13

[Robert Strong (they/them - no direct email)]

I didn't check that you copied over the resources from the current 7-zip self extractor so if you didn't please remember to do so (e.g. reshacker, etc.).

Comment 14

[Molly Howell (she/her) (mostly inactive)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #12)
> Comment on attachment 8865037 [details] [diff] [review]
> Patch
> 
> Looks good from a code perspective. You might consider using decltype as was
> done here
> bug 945192 comment #284

Couldn't use decltype because VC6 isn't aware of it and wouldn't compile.

> I assume this works with Win2K and above so we can provide unsupported
> messages. If not then you should run this by product to get their buy in.

I didn't test on Win2k, but I did build it with Visual C++ 6, so it should be working.

> Kamil has provided testing for these types of changes in the past as you can
> also see in bug 945192 and in other bugs. It would be a good thing to ask
> him if he could verify this one as well.

Kamil, would you be available to help out with verifying this? It's the same issue as bug 945192, except for the installer rather than the updater.

Comment 15

[Kamil Jozwiak [:kjozwiak]]

> Kamil, would you be available to help out with verifying this? It's the same
> issue as bug 945192, except for the installer rather than the updater.

I went through three different Win 10 machines and I couldn't reproduce the original issue so there's no way that I can verify that the installer in comment#6 has fixed the issue. Here's the process that I went through on all three Win 10 machines, please let me know if there's something that I've missed:

* downloaded the fx53 stub installer [1] and place it into the Downloads folder
* renamed the stub installer (Firefox Setup Stub 53.0.exe) into installer.exe
* launched the Process Monitor (Procmon.exe) from the Microsofts Sys Internals Suite and used the following filters
** Process Name - is - installer.exe
** Operation - is - Load Image
** Path - contains - .dll
* launched the fx53 stub installer (installer.exe) and recorded all the DLL's being loaded
* segregated all the listed DLL's into Known/Unknown categories using the WinObj.exe tool [2]
* create an empty file using on of the DLL's names [2] and place it into the Downloads folder alongside installer.exe (fx stub)
* launch installer.exe

I went through every single DLL that's listed [2] but I couldn't reproduce the issue nor did I receive an error messages of fx attempting to load the DLL from the Downloads folder. I also attempted using the original bcrypt.dll from comment#0 and the DLL names listed under comment#3 without any luck.

Matt, let me know if there's something that I've missed or of there's something else that I can try here to see if I can reproduce the original bug.

[1] https://archive.mozilla.org/pub/firefox/releases/53.0/win32/en-US/
[2] https://pastebin.mozilla.org/9021804

Comment 16

[Molly Howell (she/her) (mostly inactive)]

Load Image events only get generated when an image is successfully loaded, which means they have to be valid DLL's, empty files won't work. Remove the Operation filter and you should start seeing a few CreateFile events for DLL's in the Downloads folder.

Comment 17

[Kamil Jozwiak [:kjozwiak]]

(In reply to Matt Howell [:mhowell] from comment #16)
> Load Image events only get generated when an image is successfully loaded,
> which means they have to be valid DLL's, empty files won't work. Remove the
> Operation filter and you should start seeing a few CreateFile events for
> DLL's in the Downloads folder.

I removed the "Operation - is - Load Image" filter from procmon.exe and went through all the DLLs [1] once again on two different Win 10 x64 machines. None of the DLLs that I created alongside the fx53 stub installer were being loaded from the Downloads folder. However, OLEACCRC.DLL was always attempting to load via the Downloads folder on both machines, example:

* installer.exe - QueryOpen - C:\Users\kamil\Downloads\OLEACCRC.DLL - FAST IO DISALLOWED
* installer.exe - CreateFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - NAME NOT FOUND

Once I created OLEACCRC.DLL inside the Downloads folder and ran the fx53 stub installer, I received the following:

* QueryOpen - C:\Users\kamil\Downloads\OLEACCRC.DLL - FAST IO DISALLOWED
* CreateFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* QueryBasicInformationFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* CloseFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* IRP_MJ_CLOSE - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* CreateFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* CreateFileMapping - C:\Users\kamil\Downloads\OLEACCRC.DLL - FILE LOCKED WITH ONLY READERS
* QueryStandardInformationFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* FASTIO_RELEASE_FOR_SECTION_SYNCHRONIZATION - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* CloseFile - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS
* IRP_MJ_CLOSE - C:\Users\kamil\Downloads\OLEACCRC.DLL - SUCCESS

Using the installer from comment#6, I don't see the above behaviour and OLEACCRC.DLL isn't being listed under procmon.exe as being loaded from the Downloads folder.

Matt, as I can't reproduce the issue with the other DLL's [1], is this sufficient verification using OLEACCRC.DLL? I have two more Win 10 machines that I could go through if needed.

[1] https://pastebin.mozilla.org/9021804

Comment 18

[Molly Howell (she/her) (mostly inactive)]

It's a little curious that OLEACCRC is the only DLL you're seeing trying to load from that directory, but not curious enough for me to call it a problem, since it does always appear among the ones I see on my machines, and even one DLL is enough to open up the exploit. And everything else that you've described is what I expect to see. I'm calling that verified and requesting sec-approval. Thanks a lot for the help!

Comment 19

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8865037 [details] [diff] [review]
Patch

[Security approval request comment]
How easily could an exploit be constructed based on the patch?
I think you would pretty much have to already be aware of the attack in order to recognize what this patch is doing.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
I don't think so, the patch hardly adds any comments and the commit message is deliberately unspecific.

Which older supported branches are affected by this flaw?
All

If not all supported branches, which bug introduced the flaw?
N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
I haven't tried, but I would expect this patch to apply cleanly to all current trees, because of how infrequently this code is touched. I don't see any real risk involved there.

How likely is this patch to cause regressions; how much testing does it need?
Very unlikely; it's been manually tested, and the change is too limited to introduce much risk.

Comment 20

[Al Billings [:abillings - ex-MoCo]]

Sec-approval+ for trunk.
We'll want patches nominated for Beta and ESR52 as well, so they can land once this is on trunk.

Comment 21

[Molly Howell (she/her) (mostly inactive)]

Attachment: Patch for landing

Per comment 13, copied over the version resources. No other changes from attachment 8865037 [details] [diff] [review] as reviewed.

Comment 22

[Molly Howell (she/her) (mostly inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f5041969acc76b891db41d51141abec391f052e8
Bug 1361326 - Delay-load DLL's used by the 7-zip self-extractor. r=rstrong

Comment 23

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 8869080 [details] [diff] [review]
Patch for landing

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
This is a sec-high bug.

User impact if declined: 
Code execution as the logged-in user

Fix Landed on Version:
Nightly 55

Risk to taking this patch (and alternatives if risky): 
Very low; it's been manually tested, and the change is too limited to introduce much risk.

String or UUID changes made by this patch: 
None

[Is this code covered by automated tests?]:
No

[Has the fix been verified in Nightly?]:
Not yet, just landed to inbound

[Needs manual test from QE? If yes, steps to reproduce]:
The fix has been verified by :kjozwiak.

[List of other uplifts needed for the feature/fix]:
None

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f5041969acc7

Comment 25

[Julien Cristau [:jcristau]]

Comment on attachment 8869080 [details] [diff] [review]
Patch for landing

sec-high, beta54+, esr52+, should be in 54.0b10 and 52.2esr

Comment 26

[Molly Howell (she/her) (mostly inactive)]

https://hg.mozilla.org/releases/mozilla-beta/rev/61932206bf73a5c510f97eb3089b823b97b1141e

Comment 27

[Molly Howell (she/her) (mostly inactive)]

https://hg.mozilla.org/releases/mozilla-esr52/rev/ccbfecefae5a891c579f24d68dd07bc06e3ecde4

Comment 28

[Kamil Jozwiak [:kjozwiak]]

I went through the verification process using the STR from comment#15 and comment#17 using the following builds with Win 10 x64 Pro VM:

* fx55.0a1, buildid: 20170602030204, changeset: aeb3d0ca558f - PASSED
* fx54.0b13, buildid: 20170601133324, changeset: cb4a4275b365 - PASSED

Once the fxesr52.2 installer becomes available when we get closer to the release date, I'll go through the same verification process as mentioned above.

Comment 29

[Kamil Jozwiak [:kjozwiak]]

Liz, do you know if we have stub installers for esr builds? I took a look at the 52.2.0esr folder [1] but I couldn't find any. Previous versions don't seem to have them either which makes me wonder if stub installers are even created for esr builds. This makes verifying this issue against 52.2.0esr difficult as we need a stub installer as per the original STR form comment#0.

[1] https://archive.mozilla.org/pub/firefox/candidates/52.2.0esr-candidates/build1/

Comment 30

[Molly Howell (she/her) (mostly inactive)]

The full installer should also be vulnerable; I haven't tested it specifically, but the patched code is used in both installers.

Comment 31

[Robert Strong (they/them - no direct email)]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #29)
> Liz, do you know if we have stub installers for esr builds? I took a look at
> the 52.2.0esr folder [1] but I couldn't find any. Previous versions don't
> seem to have them either which makes me wonder if stub installers are even
> created for esr builds. This makes verifying this issue against 52.2.0esr
> difficult as we need a stub installer as per the original STR form comment#0.
and there are no stub installers for esr

Comment 32

[Kamil Jozwiak [:kjozwiak]]

Thanks :mhowell & :rstrong.

As per comment#30 and comment#31, I went through the verification process using the STR from comment#15 and comment#17 using the regular installer rather than the stub installer under Win 10 x64:

* fx52.2.0esr, buildid: 20170607123825, changeset: f68e0d98a22a

Comment 33

[Molly Howell (she/her) (mostly inactive)]

Cool; thank you once again, Kamil.