Skip to content

Commit

Permalink
[dev] fix a side-loading vulnerability with cfgmgr32.dll
Browse files Browse the repository at this point in the history
* Current Rufus and earlier versions (when compiled with MinGW) suffer from a side-loading vulnerability
  due to cfgmgr32.dll being attempted to be loaded from the same directory as the executable. This may
  result in someone being able to execute elevated malicious code if they already have gained user-level
  access to the platform and were able to drop an arbitrary cfgmgr32.dll in the same directory as rufus.
* While we were able to address similar vulnerabilities using delay-loading, this method does not appear
  to work for MinGW with this specific DLL, so we remove all the implicit CM_ function calls, that result
  in automated DLL loading that cannot be mitigated, to replace them with direct DLL hooks, which are
  not subject to Windows' default (vulnerable) DLL lookup behaviour. We still add the def for the delay
  loading in case we manage to find how to delay load cfgmgr32 with MinGW in the future...
* Fixes CVE-2025-26624 (GHSA-p8p5-r296-g2jv).
* This vulnerability was discovered by @EmperialX working with @Shauryae1337 and reported by @EmperialX.
  • Loading branch information
pbatard committed Feb 19, 2025
1 parent 50801a4 commit 622e606
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 61 deletions.
2 changes: 2 additions & 0 deletions .mingw/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ AM_V_SED = $(AM_V_SED_$(V))
# target arch. Oh, and we can't use 'target_cpu' or AC definitions on account that we are
# switching archs when building on our local machine, and don't want to have to go though
# a costly reconf each time when we can simply issue a 'make clean'.
# Oh, and to find the number after the @ sign, just have a look at the MinGW .a libraries.
TUPLE := $(shell $(CC) -dumpmachine)
TARGET := $(word 1,$(subst -, ,$(TUPLE)))
DEF_SUFFIX := $(if $(TARGET:x86_64=),.def,.def64)

.PHONY: all
# Ideally, we would also have cfgmgr32-delaylib here, but it doesn't actually delay load... :(
all: dwmapi-delaylib.lib version-delaylib.lib virtdisk-delaylib.lib wininet-delaylib.lib wintrust-delaylib.lib

%.def64: %.def
Expand Down
2 changes: 2 additions & 0 deletions .mingw/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ AM_V_SED = $(AM_V_SED_$(V))
# target arch. Oh, and we can't use 'target_cpu' or AC definitions on account that we are
# switching archs when building on our local machine, and don't want to have to go though
# a costly reconf each time when we can simply issue a 'make clean'.
# Oh, and to find the number after the @ sign, just have a look at the MinGW .a libraries.
TUPLE := $(shell $(CC) -dumpmachine)
TARGET := $(word 1,$(subst -, ,$(TUPLE)))
DEF_SUFFIX := $(if $(TARGET:x86_64=),.def,.def64)
Expand Down Expand Up @@ -367,6 +368,7 @@ uninstall-am:


.PHONY: all
# Ideally, we would also have cfgmgr32-delaylib here, but it doesn't actually delay load... :(
all: dwmapi-delaylib.lib version-delaylib.lib virtdisk-delaylib.lib wininet-delaylib.lib wintrust-delaylib.lib

%.def64: %.def
Expand Down
10 changes: 10 additions & 0 deletions .mingw/cfgmgr32.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
EXPORTS
CM_Get_Device_IDA@16
CM_Get_Device_ID_List_SizeA@12
CM_Get_Device_ID_ListA@16
CM_Locate_DevNodeA@12
CM_Get_Child@12
CM_Get_Sibling@12
CM_Get_Parent@12
CM_Get_DevNode_Status@16
CM_Get_DevNode_Registry_PropertyA@24
3 changes: 3 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ TARGET = rufus
TAGVER = $(shell git log --oneline | wc -l)
SEDCMD = s/^\([ \t]*\)Version="\([0-9]*\)\.\([0-9]*\)\.[0-9]*\.\([0-9]*\)"\(.*\)/\1Version="\2.\3.@@TAGVER@@.\4"\5/

upx: all
@upx --lzma --best src/$(TARGET)$(EXEEXT)

# This step produces the UPX compressed and signed releases that are made available for public download
# NB: UPX v3.09 or later is needed for LZMA compression (http://upx.sourceforge.net/)
release: all
Expand Down
3 changes: 3 additions & 0 deletions Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,9 @@ uninstall-am:
pdf-am ps ps-am tags tags-am uninstall uninstall-am


upx: all
@upx --lzma --best src/$(TARGET)$(EXEEXT)

# This step produces the UPX compressed and signed releases that are made available for public download
# NB: UPX v3.09 or later is needed for LZMA compression (http://upx.sourceforge.net/)
release: all
Expand Down
4 changes: 2 additions & 2 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ SUBDIRS = ../.mingw bled ext2fs ms-sys syslinux/libfat syslinux/libinstaller sys
# As far as I can tell, the following libraries are *not* vulnerable to side-loading, so we link using their regular version:
NONVULNERABLE_LIBS = -lsetupapi -lole32 -lgdi32 -lshlwapi -lcrypt32 -lcomctl32 -luuid
# The following libraries are vulnerable (or have an unknown vulnerability status), so we link using our delay-loaded replacement:
# Ideally there would also be virtdisk and wininet as delaylib's below, but the MinGW folks haven't quite sorted out delay-loading
# for x86_32 so as soon as you try to call APIs from these, the application will crash!
# See https://github.com/pbatard/rufus/issues/2272
# Oh, and don't bother trying to delay load cfgmgr32.dll, even with the DECLSPEC_IMPORT __attribute__((visibility("hidden")))
# override. It just DOESN'T BLOODY WORK and you will waste HOURS on a wild goose chase!!!
VULNERABLE_LIBS = -ldwmapi-delaylib -lversion-delaylib -lvirtdisk-delaylib -lwininet-delaylib -lwintrust-delaylib

noinst_PROGRAMS = rufus
Expand Down
4 changes: 2 additions & 2 deletions src/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,9 @@ SUBDIRS = ../.mingw bled ext2fs ms-sys syslinux/libfat syslinux/libinstaller sys
# As far as I can tell, the following libraries are *not* vulnerable to side-loading, so we link using their regular version:
NONVULNERABLE_LIBS = -lsetupapi -lole32 -lgdi32 -lshlwapi -lcrypt32 -lcomctl32 -luuid
# The following libraries are vulnerable (or have an unknown vulnerability status), so we link using our delay-loaded replacement:
# Ideally there would also be virtdisk and wininet as delaylib's below, but the MinGW folks haven't quite sorted out delay-loading
# for x86_32 so as soon as you try to call APIs from these, the application will crash!
# See https://github.com/pbatard/rufus/issues/2272
# Oh, and don't bother trying to delay load cfgmgr32.dll, even with the DECLSPEC_IMPORT __attribute__((visibility("hidden")))
# override. It just DOESN'T BLOODY WORK and you will waste HOURS on a wild goose chase!!!
VULNERABLE_LIBS = -ldwmapi-delaylib -lversion-delaylib -lvirtdisk-delaylib -lwininet-delaylib -lwintrust-delaylib
AM_V_WINDRES_0 = @echo " RC $@";$(WINDRES)
AM_V_WINDRES_1 = $(WINDRES)
Expand Down
63 changes: 46 additions & 17 deletions src/dev.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Rufus: The Reliable USB Formatting Utility
* Device detection and enumeration
* Copyright © 2014-2024 Pete Batard <[email protected]>
* Copyright © 2014-2025 Pete Batard <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -49,6 +49,21 @@ extern RUFUS_DRIVE rufus_drive[MAX_DRIVES];
extern BOOL enable_HDDs, enable_VHDs, use_fake_units, enable_vmdk, usb_debug;
extern BOOL list_non_usb_removable_drives, its_a_me_mario;

/*
* CfgMgr32.dll interface.
* Note that, unlike what is the case with other DLLs, delay-loading of cfgmgr32
* does *not* work with MinGW, so we have to go through direct hooking yet again...
*/
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Device_IDA, (DEVINST, CHAR*, ULONG, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Device_ID_List_SizeA, (PULONG, PCSTR, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Device_ID_ListA, (PCSTR, PCHAR, ULONG, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Locate_DevNodeA, (PDEVINST, DEVINSTID_A, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Child, (PDEVINST, DEVINST, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Parent, (PDEVINST, DEVINST, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_Sibling, (PDEVINST, DEVINST, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_DevNode_Status, (PULONG, PULONG, DEVINST, ULONG));
PF_TYPE_DECL(WINAPI, CONFIGRET, CM_Get_DevNode_Registry_PropertyA, (DEVINST, ULONG, PULONG, PVOID, PULONG, ULONG));

/*
* Get the VID, PID and current device speed
*/
Expand All @@ -65,15 +80,18 @@ static BOOL GetUSBProperties(char* parent_path, char* device_id, usb_device_prop
if ((parent_path == NULL) || (device_id == NULL) || (props == NULL))
goto out;

cr = CM_Locate_DevNodeA(&device_inst, device_id, 0);
PF_INIT_OR_OUT(CM_Locate_DevNodeA, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_DevNode_Registry_PropertyA, CfgMgr32);

cr = pfCM_Locate_DevNodeA(&device_inst, device_id, 0);
if (cr != CR_SUCCESS) {
uprintf("Could not get device instance handle for '%s': CR error %d", device_id, cr);
goto out;
}

props->port = 0;
size = sizeof(props->port);
cr = CM_Get_DevNode_Registry_PropertyA(device_inst, CM_DRP_ADDRESS, NULL, (PVOID)&props->port, &size, 0);
cr = pfCM_Get_DevNode_Registry_PropertyA(device_inst, CM_DRP_ADDRESS, NULL, (PVOID)&props->port, &size, 0);
if (cr != CR_SUCCESS) {
uprintf("Could not get port for '%s': CR error %d", device_id, cr);
goto out;
Expand Down Expand Up @@ -196,6 +214,8 @@ int CycleDevice(int index)
if ((index < 0) || (safe_strlen(rufus_drive[index].id) < 8))
return ERROR_INVALID_PARAMETER;

PF_INIT_OR_OUT(CM_Get_DevNode_Status, CfgMgr32);

// Need DIGCF_ALLCLASSES else disabled devices won't be listed.
dev_info = SetupDiGetClassDevsA(&GUID_DEVINTERFACE_DISK, NULL, NULL, DIGCF_PRESENT | DIGCF_ALLCLASSES);
if (dev_info == INVALID_HANDLE_VALUE) {
Expand All @@ -218,7 +238,7 @@ int CycleDevice(int index)
found = TRUE;

// Detect if the device is already disabled
if (CM_Get_DevNode_Status(&dev_status, &problem_code, dev_info_data.DevInst, 0) == CR_SUCCESS)
if (pfCM_Get_DevNode_Status(&dev_status, &problem_code, dev_info_data.DevInst, 0) == CR_SUCCESS)
disabled = (dev_status & DN_HAS_PROBLEM) && (problem_code == CM_PROB_DISABLED);

// Disable the device
Expand Down Expand Up @@ -267,7 +287,7 @@ int CycleDevice(int index)
// successful, but leave the device in an actual disabled state... So we can end up
// with zombie devices, that are effectively disabled, but that Windows still sees
// as enabled... So we need to detect this.
if (CM_Get_DevNode_Status(&dev_status, &problem_code, dev_info_data.DevInst, 0) == CR_SUCCESS) {
if (pfCM_Get_DevNode_Status(&dev_status, &problem_code, dev_info_data.DevInst, 0) == CR_SUCCESS) {
disabled = (dev_status & DN_HAS_PROBLEM) && (problem_code == CM_PROB_DISABLED);
if (disabled)
ret = ERROR_DEVICE_REINITIALIZATION_NEEDED;
Expand All @@ -278,6 +298,7 @@ int CycleDevice(int index)
SetupDiDestroyDeviceInfoList(dev_info);
if (!found)
uprintf("Could not find a device to cycle!");
out:
return ret;
}

Expand Down Expand Up @@ -473,7 +494,7 @@ BOOL GetDevices(DWORD devnum)
const char* windows_sandbox_vhd_label = "PortableBaseLayer";
// Hash table and String Array used to match a Device ID with the parent hub's Device Interface Path
htab_table htab_devid = HTAB_EMPTY;
StrArray dev_if_path;
StrArray dev_if_path = STRARRAY_EMPTY;
char letter_name[] = " (?:)";
char drive_name[] = "?:\\";
char setting_name[32];
Expand All @@ -497,6 +518,14 @@ BOOL GetDevices(DWORD devnum)
uint64_t drive_size = 0;
usb_device_props props;

PF_INIT_OR_OUT(CM_Get_Child, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_Parent, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_Sibling, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_Device_IDA, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_Device_ID_ListA, CfgMgr32);
PF_INIT_OR_OUT(CM_Get_Device_ID_List_SizeA, CfgMgr32);
PF_INIT_OR_OUT(CM_Locate_DevNodeA, CfgMgr32);

IGNORE_RETVAL(ComboBox_ResetContent(hDeviceList));
ClearDrives();
StrArrayCreate(&dev_if_path, 128);
Expand Down Expand Up @@ -526,19 +555,19 @@ BOOL GetDevices(DWORD devnum)
if (SetupDiGetDeviceInterfaceDetailA(dev_info, &devint_data, devint_detail_data, size, &size, NULL)) {

// Find the Device IDs for all the children of this hub
if (CM_Get_Child(&device_inst, dev_info_data.DevInst, 0) == CR_SUCCESS) {
if (pfCM_Get_Child(&device_inst, dev_info_data.DevInst, 0) == CR_SUCCESS) {
device_id[0] = 0;
s = StrArrayAdd(&dev_if_path, devint_detail_data->DevicePath, TRUE);
uuprintf(" Hub[%d] = '%s'", s, devint_detail_data->DevicePath);
if ((s>= 0) && (CM_Get_Device_IDA(device_inst, device_id, MAX_PATH, 0) == CR_SUCCESS)) {
if ((s>= 0) && (pfCM_Get_Device_IDA(device_inst, device_id, MAX_PATH, 0) == CR_SUCCESS)) {
ToUpper(device_id);
if ((k = htab_hash(device_id, &htab_devid)) != 0) {
htab_devid.table[k].data = (void*)(uintptr_t)s;
}
uuprintf(" Found ID[%03d]: %s", k, device_id);
while (CM_Get_Sibling(&device_inst, device_inst, 0) == CR_SUCCESS) {
while (pfCM_Get_Sibling(&device_inst, device_inst, 0) == CR_SUCCESS) {
device_id[0] = 0;
if (CM_Get_Device_IDA(device_inst, device_id, MAX_PATH, 0) == CR_SUCCESS) {
if (pfCM_Get_Device_IDA(device_inst, device_id, MAX_PATH, 0) == CR_SUCCESS) {
ToUpper(device_id);
if ((k = htab_hash(device_id, &htab_devid)) != 0) {
htab_devid.table[k].data = (void*)(uintptr_t)s;
Expand Down Expand Up @@ -566,7 +595,7 @@ BOOL GetDevices(DWORD devnum)
// Also compute the uasp_start index
if (strcmp(usbstor_name[s], "UASPSTOR") == 0)
uasp_start = s;
if (CM_Get_Device_ID_List_SizeA(&list_size[s], usbstor_name[s], ulFlags) != CR_SUCCESS)
if (pfCM_Get_Device_ID_List_SizeA(&list_size[s], usbstor_name[s], ulFlags) != CR_SUCCESS)
list_size[s] = 0;
if (list_size[s] != 0)
full_list_size += list_size[s]-1; // remove extra NUL terminator
Expand Down Expand Up @@ -601,7 +630,7 @@ BOOL GetDevices(DWORD devnum)
for (s = 0, i = 0; s < ARRAYSIZE(usbstor_name); s++) {
list_start[s] = i;
if (list_size[s] > 1) {
if (CM_Get_Device_ID_ListA(usbstor_name[s], &devid_list[i], list_size[s], ulFlags) != CR_SUCCESS)
if (pfCM_Get_Device_ID_ListA(usbstor_name[s], &devid_list[i], list_size[s], ulFlags) != CR_SUCCESS)
continue;
if (usb_debug) {
uprintf("Processing IDs belonging to '%s':", usbstor_name[s]);
Expand Down Expand Up @@ -708,17 +737,17 @@ BOOL GetDevices(DWORD devnum)
// a lookup table, but there shouldn't be that many USB storage devices connected...
// NB: Each of these Device IDs should have a child, from which we get the Device Instance match.
for (device_id = devid_list; *device_id != 0; device_id += strlen(device_id) + 1) {
if (CM_Locate_DevNodeA(&parent_inst, device_id, 0) != CR_SUCCESS) {
if (pfCM_Locate_DevNodeA(&parent_inst, device_id, 0) != CR_SUCCESS) {
uuprintf("Could not locate device node for '%s'", device_id);
continue;
}
if (CM_Get_Child(&device_inst, parent_inst, 0) != CR_SUCCESS) {
if (pfCM_Get_Child(&device_inst, parent_inst, 0) != CR_SUCCESS) {
uuprintf("Could not get children of '%s'", device_id);
continue;
}
if (device_inst != dev_info_data.DevInst) {
// Try the siblings
while (CM_Get_Sibling(&device_inst, device_inst, 0) == CR_SUCCESS) {
while (pfCM_Get_Sibling(&device_inst, device_inst, 0) == CR_SUCCESS) {
if (device_inst == dev_info_data.DevInst) {
uuprintf("NOTE: Matched instance from sibling for '%s'", device_id);
break;
Expand Down Expand Up @@ -759,8 +788,8 @@ BOOL GetDevices(DWORD devnum)
// for UASP devices in ASUS "Turbo Mode" or "Apple Mobile Device USB Driver" for iPods)
// so try to see if we can match the grandparent.
if ( ((uintptr_t)htab_devid.table[j].data == 0)
&& (CM_Get_Parent(&grandparent_inst, parent_inst, 0) == CR_SUCCESS)
&& (CM_Get_Device_IDA(grandparent_inst, str, MAX_PATH, 0) == CR_SUCCESS) ) {
&& (pfCM_Get_Parent(&grandparent_inst, parent_inst, 0) == CR_SUCCESS)
&& (pfCM_Get_Device_IDA(grandparent_inst, str, MAX_PATH, 0) == CR_SUCCESS) ) {
device_id = str;
method_str = "[GP]";
ToUpper(device_id);
Expand Down
32 changes: 4 additions & 28 deletions src/dev.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Rufus: The Reliable USB Formatting Utility
* Device detection and enumeration
* Copyright © 2014-2019 Pete Batard <[email protected]>
* Copyright © 2014-2025 Pete Batard <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -46,20 +46,6 @@ typedef struct usb_device_props {
/*
* Windows DDK API definitions. Most of it copied from MinGW's includes
*/
typedef DWORD DEVNODE, DEVINST;
typedef DEVNODE *PDEVNODE, *PDEVINST;
typedef DWORD RETURN_TYPE;
typedef RETURN_TYPE CONFIGRET;
typedef CHAR *DEVINSTID_A;

#ifndef CM_GETIDLIST_FILTER_PRESENT
#define CM_GETIDLIST_FILTER_PRESENT 0x00000100
#endif

#ifndef FILE_DEVICE_USB
#define FILE_DEVICE_USB FILE_DEVICE_UNKNOWN
#endif

typedef enum USB_CONNECTION_STATUS {
NoDeviceConnected,
DeviceConnected,
Expand All @@ -77,26 +63,16 @@ typedef enum USB_HUB_NODE {
UsbMIParent
} USB_HUB_NODE;

/* Cfgmgr32.dll interface */
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Device_IDA(DEVINST dnDevInst, CHAR* Buffer, ULONG BufferLen, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Device_ID_List_SizeA(PULONG pulLen, PCSTR pszFilter, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Device_ID_ListA(PCSTR pszFilter, PCHAR Buffer, ULONG BufferLen, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Locate_DevNodeA(PDEVINST pdnDevInst, DEVINSTID_A pDeviceID, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Child(PDEVINST pdnDevInst, DEVINST dnDevInst, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Parent(PDEVINST pdnDevInst, DEVINST dnDevInst, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_Sibling(PDEVINST pdnDevInst, DEVINST dnDevInst, ULONG ulFlags);
DECLSPEC_IMPORT CONFIGRET WINAPI CM_Get_DevNode_Status(PULONG pulStatus, PULONG pulProblemNumber, DEVINST dnDevInst, ULONG ulFlags);

#define USB_HUB_CYCLE_PORT 273
#define USB_GET_NODE_CONNECTION_INFORMATION_EX 274
#define USB_GET_NODE_CONNECTION_INFORMATION_EX_V2 279

#define IOCTL_USB_HUB_CYCLE_PORT \
CTL_CODE(FILE_DEVICE_USB, USB_HUB_CYCLE_PORT, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, USB_HUB_CYCLE_PORT, METHOD_BUFFERED, FILE_ANY_ACCESS)
#define IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX \
CTL_CODE(FILE_DEVICE_USB, USB_GET_NODE_CONNECTION_INFORMATION_EX, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, USB_GET_NODE_CONNECTION_INFORMATION_EX, METHOD_BUFFERED, FILE_ANY_ACCESS)
#define IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX_V2 \
CTL_CODE(FILE_DEVICE_USB, USB_GET_NODE_CONNECTION_INFORMATION_EX_V2, METHOD_BUFFERED, FILE_ANY_ACCESS)
CTL_CODE(FILE_DEVICE_UNKNOWN, USB_GET_NODE_CONNECTION_INFORMATION_EX_V2, METHOD_BUFFERED, FILE_ANY_ACCESS)

/* Most of the structures below need to be packed */
#pragma pack(push, 1)
Expand Down
24 changes: 18 additions & 6 deletions src/rufus.c
Original file line number Diff line number Diff line change
Expand Up @@ -3257,19 +3257,31 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
{0, 0, NULL, 0}
};

// Disable loading system DLLs from the current directory (sideloading mitigation)
// Disable loading system DLLs from the current directory (side-loading mitigation)
// PS: You know that official MSDN documentation for SetDllDirectory() that explicitly
// indicates that "If the parameter is an empty string (""), the call removes the current
// directory from the default DLL search order"? Yeah, that doesn't work. At all.
// Still, we invoke it, for platforms where the following call might actually work...
SetDllDirectoryA("");
// directory from the default DLL search order"? Yeah, that doesn't work. At all. And as
// a matter of fact, Microsoft has now altered their doc to remove that part, though it
// is still *currently* being mentioned in their doc for Dynamic-Link Library Security:
// https://web.archive.org/web/20250206201109/https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-security
// So, Microsoft currently offers NO WAY to easily disable the main vulnerability most
// applications suffer from, which is the loading of bloody DLLs from the current/app
// dir, even for executables, like Rufus, that are designed from the get go to NEVER EVER
// rely on any DLLs there, and would like to DISABLE THIS UTTER BULLSHIT OF AN ENTIRELY
// PREVENTABLE SECURITY RISK! The end result of all this is that we have to contend with
// delay loading (*when* it actually works) or direct hooking (when it doesn't) and no
// longer try to bother with a quick and easy side-loading fix that Microsoft has been
// dangling as a lure, for years, but hasn't actually bothered to implement...
// SetDllDirectoryA("");

// 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).
// the linker properties in Visual Studio... which means this won't work with MinGW.
// For all other DLLs, use SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32),
// though this *STILL* does not prevent the Windows default of looking for DLLs in the
// current directories.
SetDefaultDllDirectories(LOAD_LIBRARY_SEARCH_SYSTEM32);

uprintf("*** " APPLICATION_NAME " init ***\n");
Expand Down
Loading

0 comments on commit 622e606

Please sign in to comment.