mirror of https://github.com/pbatard/rufus.git
[msvc] add provision to prevent DLL search order hijacking through delay loading
* Hypothetically if the user's current directory contains a malicious DLL that DLL could be loaded instead of the one in System32. * Whereas the previous patch should have taken care of the one DLL referenced by Rufus that may be vulnerable to this attack (version.dll), we nonetheless add delay loading for all the libraries we reference as a precautionary measure. * One can confirm that this works by using dumpbin.exe /IMPORTS to make sure a specific DLL is delay loaded. Then putting a breakpoint in the delay load hook should also confirm that the hook is used. * Closes #1838
This commit is contained in:
Austin Wise
Pete Batard
parent
f6ac559f4d
commit
ef2ff7179d
|
|
@ -133,11 +133,12 @@
|
|||
<AdditionalOptions>/utf-8 $(ExternalCompilerOptions) %(AdditionalOptions)</AdditionalOptions>
|
||||
</ClCompile>
|
||||
<Link>
|
||||
<AdditionalDependencies>advapi32.lib;comctl32.lib;comdlg32.lib;crypt32.lib;gdi32.lib;ole32.lib;setupapi.lib;shell32.lib;shlwapi.lib;wintrust.lib;psapi.lib;%(AdditionalDependencies)</AdditionalDependencies>
|
||||
<AdditionalDependencies>advapi32.lib;comctl32.lib;comdlg32.lib;crypt32.lib;gdi32.lib;ole32.lib;setupapi.lib;shell32.lib;shlwapi.lib;wintrust.lib;%(AdditionalDependencies)</AdditionalDependencies>
|
||||
<UACExecutionLevel>RequireAdministrator</UACExecutionLevel>
|
||||
<GenerateDebugInformation>true</GenerateDebugInformation>
|
||||
<SubSystem>Windows</SubSystem>
|
||||
<TargetMachine>MachineX86</TargetMachine>
|
||||
<DelayLoadDLLs>advapi32.dll;comctl32.dll;comdlg32.dll;crypt32.dll;gdi32.dll;ole32.dll;setupapi.dll;shell32.dll;shlwapi.dll;wintrust.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
|
||||
</Link>
|
||||
<ResourceCompile>
|
||||
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
|
||||
|
|
@ -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>advapi32.dll;comctl32.dll;comdlg32.dll;crypt32.dll;gdi32.dll;ole32.dll;setupapi.dll;shell32.dll;shlwapi.dll;wintrust.dll;ole32.dll;advapi32.dll;gdi32.dll;shell32.dll;comdlg32.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
|
||||
</Link>
|
||||
<ResourceCompile>
|
||||
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
|
||||
|
|
@ -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>advapi32.dll;comctl32.dll;comdlg32.dll;crypt32.dll;gdi32.dll;ole32.dll;setupapi.dll;shell32.dll;shlwapi.dll;wintrust.dll;ole32.dll;advapi32.dll;gdi32.dll;shell32.dll;comdlg32.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
|
||||
</Link>
|
||||
<ResourceCompile>
|
||||
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
|
||||
|
|
@ -214,11 +217,12 @@
|
|||
<AdditionalOptions>/utf-8 $(ExternalCompilerOptions) %(AdditionalOptions)</AdditionalOptions>
|
||||
</ClCompile>
|
||||
<Link>
|
||||
<AdditionalDependencies>advapi32.lib;comctl32.lib;comdlg32.lib;crypt32.lib;gdi32.lib;ole32.lib;setupapi.lib;shell32.lib;shlwapi.lib;wintrust.lib;psapi.lib;%(AdditionalDependencies)</AdditionalDependencies>
|
||||
<AdditionalDependencies>advapi32.lib;comctl32.lib;comdlg32.lib;crypt32.lib;gdi32.lib;ole32.lib;setupapi.lib;shell32.lib;shlwapi.lib;wintrust.lib;%(AdditionalDependencies)</AdditionalDependencies>
|
||||
<UACExecutionLevel>RequireAdministrator</UACExecutionLevel>
|
||||
<GenerateDebugInformation>true</GenerateDebugInformation>
|
||||
<SubSystem>Windows</SubSystem>
|
||||
<TargetMachine>MachineX64</TargetMachine>
|
||||
<DelayLoadDLLs>advapi32.dll;comctl32.dll;comdlg32.dll;crypt32.dll;gdi32.dll;ole32.dll;setupapi.dll;shell32.dll;shlwapi.dll;wintrust.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
|
||||
</Link>
|
||||
<ResourceCompile>
|
||||
<PreprocessorDefinitions>_UNICODE;UNICODE;RUFUS_LOC;%(PreprocessorDefinitions)</PreprocessorDefinitions>
|
||||
|
|
|
|||
39
src/rufus.c
39
src/rufus.c
|
|
@ -36,6 +36,7 @@
|
|||
#include <io.h>
|
||||
#include <getopt.h>
|
||||
#include <assert.h>
|
||||
#include <delayimp.h>
|
||||
|
||||
#include "rufus.h"
|
||||
#include "missing.h"
|
||||
|
|
@ -3191,6 +3192,23 @@ 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;
|
||||
}
|
||||
|
||||
#if defined(_MSC_VER)
|
||||
// By default the Windows SDK headers have a `const` while MinGW does not.
|
||||
const
|
||||
#endif
|
||||
PfnDliHook __pfnDliNotifyHook2 = dllDelayLoadHook;
|
||||
|
||||
/*
|
||||
* Application Entrypoint
|
||||
|
|
@ -3202,7 +3220,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;
|
||||
|
|
@ -3238,22 +3255,18 @@ 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, we can delay load the DLL and use a delay load
|
||||
// hook to load them from System32. Note that, for this to work, something like:
|
||||
// 'somelib.dll;%(DelayLoadDLLs)' must be added to the 'Delay Loaded Dlls' option of
|
||||
// the linker properties in Visual Studio (which means this won't work with MinGW).
|
||||
// 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);
|
||||
|
||||
|
|
|
|||
10
src/rufus.rc
10
src/rufus.rc
|
|
@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
|
|||
IDD_DIALOG DIALOGEX 12, 12, 232, 326
|
||||
STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
|
||||
EXSTYLE WS_EX_ACCEPTFILES
|
||||
CAPTION "Rufus 3.18.1857"
|
||||
CAPTION "Rufus 3.18.1858"
|
||||
FONT 9, "Segoe UI Symbol", 400, 0, 0x0
|
||||
BEGIN
|
||||
LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP
|
||||
|
|
@ -395,8 +395,8 @@ END
|
|||
//
|
||||
|
||||
VS_VERSION_INFO VERSIONINFO
|
||||
FILEVERSION 3,18,1857,0
|
||||
PRODUCTVERSION 3,18,1857,0
|
||||
FILEVERSION 3,18,1858,0
|
||||
PRODUCTVERSION 3,18,1858,0
|
||||
FILEFLAGSMASK 0x3fL
|
||||
#ifdef _DEBUG
|
||||
FILEFLAGS 0x1L
|
||||
|
|
@ -414,13 +414,13 @@ BEGIN
|
|||
VALUE "Comments", "https://rufus.ie"
|
||||
VALUE "CompanyName", "Akeo Consulting"
|
||||
VALUE "FileDescription", "Rufus"
|
||||
VALUE "FileVersion", "3.18.1857"
|
||||
VALUE "FileVersion", "3.18.1858"
|
||||
VALUE "InternalName", "Rufus"
|
||||
VALUE "LegalCopyright", "© 2011-2021 Pete Batard (GPL v3)"
|
||||
VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html"
|
||||
VALUE "OriginalFilename", "rufus-3.18.exe"
|
||||
VALUE "ProductName", "Rufus"
|
||||
VALUE "ProductVersion", "3.18.1857"
|
||||
VALUE "ProductVersion", "3.18.1858"
|
||||
END
|
||||
END
|
||||
BLOCK "VarFileInfo"
|
||||
|
|
|
|||
Loading…
Reference in New Issue