Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Fix vunerablity to DLL Search Order Hijacking by delay loading version.dll #1838

Closed
wants to merge 2 commits into from
Closed

Fix vunerablity to DLL Search Order Hijacking by delay loading version.dll #1838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7712ad6
Fix vunerablity to DLL Search Order Hijacking by delay loading versio…
AustinWise Dec 12, 2021
71b8d01
Fix compile error on MinGW.
AustinWise Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .vs/rufus.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
<GenerateDebugInformation>true</GenerateDebugInformation>
<SubSystem>Windows</SubSystem>
<TargetMachine>MachineX86</TargetMachine>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand All @@ -162,6 +163,7 @@
<GenerateDebugInformation>true</GenerateDebugInformation>
<SubSystem>Windows</SubSystem>
<AdditionalLibraryDirectories>C:\Program Files (x86)\Windows Kits\10\Lib\10.0.15063.0\um\arm</AdditionalLibraryDirectories>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand All @@ -188,6 +190,7 @@
<GenerateDebugInformation>true</GenerateDebugInformation>
<SubSystem>Windows</SubSystem>
<AdditionalLibraryDirectories>C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\um\arm64</AdditionalLibraryDirectories>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down Expand Up @@ -219,6 +222,7 @@
<GenerateDebugInformation>true</GenerateDebugInformation>
<SubSystem>Windows</SubSystem>
<TargetMachine>MachineX64</TargetMachine>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down Expand Up @@ -246,6 +250,7 @@
<SubSystem>Windows</SubSystem>
<TargetMachine>MachineX86</TargetMachine>
<AdditionalOptions>/BREPRO %(AdditionalOptions)</AdditionalOptions>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down Expand Up @@ -273,6 +278,7 @@
<SubSystem>Windows</SubSystem>
<AdditionalLibraryDirectories>C:\Program Files (x86)\Windows Kits\10\Lib\10.0.15063.0\um\arm</AdditionalLibraryDirectories>
<AdditionalOptions>/BREPRO %(AdditionalOptions)</AdditionalOptions>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down Expand Up @@ -302,6 +308,7 @@
<SubSystem>Windows</SubSystem>
<AdditionalLibraryDirectories>C:\Program Files (x86)\Windows Kits\10\Lib\10.0.16299.0\um\arm64</AdditionalLibraryDirectories>
<AdditionalOptions>/BREPRO %(AdditionalOptions)</AdditionalOptions>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down Expand Up @@ -334,6 +341,7 @@
<SubSystem>Windows</SubSystem>
<TargetMachine>MachineX64</TargetMachine>
<AdditionalOptions>/BREPRO %(AdditionalOptions)</AdditionalOptions>
<DelayLoadDLLs>version.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
</Link>
<ResourceCompile>
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
Expand Down
40 changes: 27 additions & 13 deletions src/rufus.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <io.h>
#include <getopt.h>
#include <assert.h>
#include <delayimp.h>

#include "rufus.h"
#include "missing.h"
Expand Down Expand Up @@ -3193,6 +3194,26 @@ static HANDLE SetHogger(void)
return hogmutex;
}

// For delay-loaded DLLs,
// use LOAD_LIBRARY_SEARCH_SYSTEM32 to avoid DLL search order hijacking.
FARPROC WINAPI dllDelayLoadHook(unsigned dliNotify, PDelayLoadInfo pdli)
{
if (dliNotify == dliNotePreLoadLibrary)
{
// Windows 7 without KB2533623 does not support the LOAD_LIBRARY_SEARCH_SYSTEM32 flag.
// That is is OK, because the delay load handler will interrupt the NULL return value
// to mean that it should perform a normal LoadLibrary.
return (FARPROC)LoadLibraryExA(pdli->szDll, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
}

return NULL;
}

#ifdef _MSC_VER
// For some reason the Windows SDK headers have a `const` while MinGW does not.
const
#endif
PfnDliHook __pfnDliNotifyHook2 = dllDelayLoadHook;

/*
* Application Entrypoint
Expand All @@ -3204,7 +3225,6 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
#endif
{
const char* rufus_loc = "rufus.loc";
wchar_t kernel32_path[MAX_PATH];
int i, opt, option_index = 0, argc = 0, si = 0, lcid = GetUserDefaultUILanguage();
int wait_for_mutex = 0;
FILE* fd;
Expand Down Expand Up @@ -3240,22 +3260,16 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
// Still, we invoke it, for platforms where the following call might actually work...
SetDllDirectoryA("");

// Also, even if you use SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32), you're
// still going to be brought down if you link to wininet.lib or dwmapi.lib, as these two
// perform their DLL invocations before you've had a chance to execute anything.
// Of course, this is not something that security "researchers" will bother looking into
// to try to help fellow developers, when they can get an ego fix by simply throwing
// generic URLs around and deliberately refusing to practice *responsible disclosure*...
// For libraries on the KnownDLLs list, the system will always load them from System32.
// For other DLLs we link directly to, like version.dll, we delay load the DLL and use
// a delay load hook to load them from System32.
// For all other DLLs, use SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32).
// Finally, we need to perform the whole gymkhana below, where we can't call on
// SetDefaultDllDirectories() directly, because Windows 7 doesn't have the API exposed.
GetSystemDirectoryW(kernel32_path, ARRAYSIZE(kernel32_path));
wcsncat(kernel32_path, L"\\kernel32.dll", ARRAYSIZE(kernel32_path) - wcslen(kernel32_path) - 1);
// NB: Because kernel32 should already be loaded, what we do above to ensure that we
// (re)pick the system one is mostly unnecessary. But since for a hammer everything is a
// nail... Also, no, Coverity, we never need to care about freeing kernel32 as a library.
// Also, no, Coverity, we never need to care about freeing kernel32 as a library.
// coverity[leaked_storage]
pfSetDefaultDllDirectories = (SetDefaultDllDirectories_t)
GetProcAddress(LoadLibraryW(kernel32_path), "SetDefaultDllDirectories");
GetProcAddress(LoadLibraryW(L"kernel32.dll"), "SetDefaultDllDirectories");
if (pfSetDefaultDllDirectories != NULL)
pfSetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32);

Expand Down