Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report of DLL Hijacking vulnerability causing arbitrary code execution and privilege escalation #1877

Closed
ScriptIdiot opened this issue Feb 24, 2022 · 14 comments
Assignees
Milestone

Comments

@ScriptIdiot
Copy link

ScriptIdiot commented Feb 24, 2022

Vulnerable Software and Version:

  1. Rufus 3.17.1846 executable
  2. Rufus 3.17.1846 portable executable

Vulnerable software download link:

https://rufus.ie/en/
https://github.com/pbatard/rufus/releases/tag/v3.17

Date discovered and reported:

25 Feb 2022

Description:

Both Rufus 3.17.1846 executable AND portable executable are suffering from CWD DLL Hijacking by placing x86 MSASN1.dll or VERSION.dll in the current directory as the executables, which could cause arbitrary code execution and privilege escalation.

Taking MSASN1.dll as an example, craft a malicious x86 DLL with an entry point with DllMain and place in the current directory, once double click the executable, an x86 admin shell could be obtained as the executable requires admin right to run by design.

Attack vector:

Taking MSASN1.dll as an example
PoC code of dll can be found in my repository

Attack steps:

  1. Craft and drop a malicious DLL named as "MSASN1.dll" with entry point DllMain
    image

  2. Double click the executable "Rufus", administrator privilege is required to run

  3. Malicious DLL has been called and an admin shell can be obtained as PoC
    image

PoC code can be found here:
https://github.com/ScriptIdiot/Dll-Hijacking-Rufus-3.17.1846

@ScriptIdiot ScriptIdiot changed the title Report of DLL Hijacking vulnerability cause arbitrary code execution and privilege escalation Report of DLL Hijacking vulnerability causing arbitrary code execution and privilege escalation Feb 24, 2022
@pbatard
Copy link
Owner

pbatard commented Feb 24, 2022

I believe this or part of this should have been fixed in f6ac559 and ef2ff71 as we got the report of the vulnerability with version.dll in #1838. Not sure from which dependency the ASN1 DLL comes into play though.

Of course, I'll take a look at it myself, but can you please test with the latest builds that you'll find in the artifacts from https://github.com/pbatard/rufus/actions/runs/1878105382 and https://github.com/pbatard/rufus/actions/runs/1878105381.

I would expect the VS2022 builds to no longer be vulnerable to DLL sideloading on account of ef2ff71, and I also believe that you should no longer be able to use version.dll with the MinGW version, though it's possible that MSASN1.dll will still work there.

@ScriptIdiot
Copy link
Author

ScriptIdiot commented Feb 24, 2022

image

Just checked the X86 version, Rufus 3.18.1873 (Alpha) , its suffering from the hijacking of dll named as MSASN1, will try for others as well

Just checked x64 version as well, Rufus 3.18.1873 (Alpha),
image

@pbatard
Copy link
Owner

pbatard commented Feb 24, 2022

Thanks.

So that means that the "delay load" mitigation that we applied to VS builds does not work as expected... That's quite disappointing, but then again, since we can't do that for MinGW builds, it wasn't meant to be used as a bulletproof method to avoid sideloading.

@pbatard pbatard self-assigned this Feb 24, 2022
@pbatard pbatard added this to the 3.18 milestone Feb 24, 2022
@ScriptIdiot
Copy link
Author

ScriptIdiot commented Feb 24, 2022

It would be great if CVE can be assigned on DLL Hijacking vulnerability found in both Rufus 3.17.1846 executable and
Rufus 3.17.1846 portable executable by hijacking MSASN1.dll, thanks!

@pbatard
Copy link
Owner

pbatard commented Feb 24, 2022

I'll leave it up to you to decide whether you want to report it as a CVE.

I plan on addressing this as soon as I can, and I'm hoping to release Rufus 3.18 in the next couple of weeks, but I will leave it up to you on whether you want to publicize this vulnerability further (knowing that the 'version.dll' vulnerability of Rufus 3.17 had already been publicized in #1838 anyway, without a CVE having been created).

pbatard added a commit that referenced this issue Feb 25, 2022
* ef2ff71 was supposed to apply delay loading to our DLLs, for all MSVC builds,
  thereby preventing sideloading attacks, but the patch actually only set the DelayLoadDLLs
  property for Debug builds and not Release builds, with the result that side loading could
  still be triggered for the Release executables, as demonstrated in #1877.
* This patch therefore properly sets the DelayLoadDLLs for all builds, which should take care
  of the side loading vulnerability at least for MSVC executables.
* A subsequent patch will still be needed for MinGW, since there is no equivalent to DelayLoadDLLs.
* This addresses part of #1877.
@pbatard
Copy link
Owner

pbatard commented Feb 25, 2022

Okay, looking at it more closely, I found out that DLL delay loading, that was introduced in ef2ff71, was only applied to MSVC Debug builds and not the Release builds, which explains why the MSVC artifacts were still subject to side loading. With the new patch applied above, you should find that the artifacts from https://github.com/pbatard/rufus/actions/runs/1900092903 are no longer subject to the vulnerability.

Of course, that still doesn't address the issue for MinGW, which I'm planning to do next (since only the Windows Store version is built with MSVC whereas the executables from https://rufus.ie are build with MinGW).

From what I can see (here and here), msasn1.dll is picked from wintrust, which means that I may have to drop linking with wintrust.lib altogether, which might require some work...

@assarbad
Copy link

assarbad commented Feb 28, 2022

So that means that the "delay load" mitigation that we applied to VS builds does not work as expected... That's quite disappointing, but then again, since we can't do that for MinGW builds, it wasn't meant to be used as a bulletproof method to avoid sideloading.

@pbatard does this method not work for you? It's a relatively new feature, though. But Clang should also be able to offer the functionality with clang-cl and lld-link.

@pbatard
Copy link
Owner

pbatard commented Feb 28, 2022

@assarbad, that's good info, though it might be a bit cumbersome to set up. I'll give it a whirl, as I'd sure prefer to stop the madness of having to hook into almost every DLL manually...

@pbatard
Copy link
Owner

pbatard commented Mar 3, 2022

Since there's only one API call we pick from WinTrust, I'm going to continue going with direct DLL hooking as a workaround for the time being. So the next patch will close this issue and you should no find that none of the executables builds from GitHub Actions should be subject to DLL hijacking.

I have however made a note to explore delay loading for MinGW for the 3.19 release of Rufus, as I obviously don't want to end up in the situation where hooking into new DLLs makes us vulnerable to sideloading again, and I also wouldn't mind just being able to call DLL APIs without having to do the whole direct hook gymnastics.

@pbatard pbatard closed this as completed in 1947266 Mar 3, 2022
@pbatard
Copy link
Owner

pbatard commented Mar 3, 2022

Btw, feel free to test the newly released Rufus 3.18 BETA for DLL hijacking, and let me know if you still see anything amiss there.

@ScriptIdiot
Copy link
Author

ScriptIdiot commented Mar 4, 2022

@pbatard Just tested against Rufus 3.18 BETA, dont think its vulnerable now, thanks for the update!

pbatard added a commit that referenced this issue Apr 12, 2022
* This reverts much of commits f6ac559 and 1947266
  so that we call the Windows APIs directly again, while ensuring that, by the time we load the DLLs,
  sideloading mitigation has already been applied by the application.
* This is a continuation of #1877, and should help prevent re-introducing side-loading issues when we
  link against new libraries, as well as allow us to drop some of the manual DLL hooking we've been
  doing to prevent it, to clean up the code.
* Note that this is a bit more complex than what the stackoverflow post suggests, because we need to
  create delayloaded libs for both 32-bit and 64-bit, which use a different calling convention and
  therefore need to use different .def files. So there's a lot of gymkhana involved, with Makefiles
  and whatnot, to get us there.
* Also simplify the use of CM_Get_DevNode_Registry_PropertyA() in dev.c since recent versions of
  MinGW now have support for it.
* Also fix 2 small issues in net.c (potential overflow) and format.c (memory leak).
@pbatard
Copy link
Owner

pbatard commented Apr 12, 2022

does this method not work for you?

It works, and I have just applied a patch where we use dlltool --output-delaylib ... for MinGW so that we can simplify the code by not having to hook into DLLs manually to prevent side loading, but the dlltool approach is more painful than the stackoverflow post makes it look, because 32-bit and 64-bit use different calling conventions, and therefore they cannot use the same .def (the 32-bit version requires stdcall @## decorations where the 64-bit version just uses plain cdecl undecorated names). So we have to perform extra processing of the .def depending on the target arch, and, whereas the end result looks simple, it was a lot of trial and error to get there.

Anyway, moving forward, when linking with new libraries may be subject to side-loading before the application gets a chance to restrict the loading of DLLs to only system directories, we should be able to just add them to the existing MSVC and MinGW mitigation framework, so that delay-loading is used, which should hopefully keep us safe.

@pbatard
Copy link
Owner

pbatard commented Apr 26, 2022

Yet one more entry in the never-ending story of DLL side-loading prevention.

Unfortunately, it looks like the MinGW method of delay-loading is not yet up to par with the MSVC one, because if you delay-load wininet with MinGW (which we do in 3194a4d), then Rufus exits before it launches its UI (and does so through no code that we have control of)...

I guess the MinGW devs will have to weather-test their delay-loading some more, because (at least for wininet) you get a literal "That's it, I'm outta here" from the runtime when it encounters an issue while trying to delay-load the DLL.

I kind of suspect that this might have to do with the fact that we start a separate thread with some networking tasks on startup, and the current MinGW delay-load hooks may not like that.

At any rate, it looks like I'm going to revert commit 3194a4d yet again (which is annoying, since net.c was the one place that greatly benefited from no longer having to hook directly into the DLL), so that the MinGW version of Rufus can actually launch.

What a massive pain in the ass this DLL side-loading mitigation continues to be!

pbatard added a commit that referenced this issue Apr 27, 2022
* This reverts 3194a4d on account that MinGW's delay loading of
  wininet.dll causes the application to prematurely close.
* Yet another episode of the never ending #1877 saga...
@github-actions
Copy link

github-actions bot commented Aug 4, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants