Windows: Restrict path for dynamic library loading

The LoadLibraryA() function performs a search through various paths when
provided a library name that does not include a path element. All of the
libraries that libusb needs to dynamically load are installed in the
Windows system directory, thus it is not necessary to search any paths.

To harden the security of libusb and prevent loading libraries from
outside of the system directory should an attacker be able to alter the
environment or place an identically named library somewhere in the
search paths that occur before the Windows system directory, introduce a
function that calls LoadLibraryA() with a full path to the requested
library witin the Windows system directory. Note that we cannot call
SetDllDirectoryA() because as a library we should not alter the DLL
search path of the application. We also cannot use LoadLibraryExA() with
the LOAD_LIBRARY_SEARCH_* flags because those flags require a specific
security update to be installed on Vista and 7.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/os/windows_common.c b/libusb/os/windows_common.c
index c72c4d2..f91075a 100644 (file)
--- a/libusb/os/windows_common.c
+++ b/libusb/os/windows_common.c
@@ -95,6 +95,35 @@ const char *windows_error_str(DWORD error_code)
 }
 #endif
 
+/*
+ * Dynamically loads a DLL from the Windows system directory.  Unlike the
+ * LoadLibraryA() function, this function will not search through any
+ * directories to try and find the library.
+ */
+HMODULE load_system_library(struct libusb_context *ctx, const char *name)
+{
+       char library_path[MAX_PATH];
+       char *filename_start;
+       UINT length;
+
+       length = GetSystemDirectoryA(library_path, sizeof(library_path));
+       if ((length == 0) || (length >= (UINT)sizeof(library_path))) {
+               usbi_err(ctx, "program assertion failed - could not get system directory");
+               return NULL;
+       }
+
+       filename_start = library_path + length;
+       // Append '\' + name + ".dll" + NUL
+       length += 1 + strlen(name) + 4 + 1;
+       if (length >= (UINT)sizeof(library_path)) {
+               usbi_err(ctx, "program assertion failed - library path buffer overflow");
+               return NULL;
+       }
+
+       sprintf(filename_start, "\\%s.dll", name);
+       return LoadLibraryA(library_path);
+}
+
 /* Hash table functions - modified From glibc 2.3.2:
    [Aho,Sethi,Ullman] Compilers: Principles, Techniques and Tools, 1986
    [Knuth]            The Art of Computer Programming, part 3 (6.4)  */

diff --git a/libusb/os/windows_common.h b/libusb/os/windows_common.h
index 9f88956..0c4b94c 100644 (file)
--- a/libusb/os/windows_common.h
+++ b/libusb/os/windows_common.h
@@ -60,29 +60,29 @@
  * API macros - leveraged from libusb-win32 1.x
  */
 #define DLL_STRINGIFY(s) #s
-#define DLL_LOAD_LIBRARY(name) LoadLibraryA(DLL_STRINGIFY(name))
 
 /*
  * Macros for handling DLL themselves
  */
 #define DLL_HANDLE_NAME(name) __dll_##name##_handle
 
-#define DLL_DECLARE_HANDLE(name)                               \
+#define DLL_DECLARE_HANDLE(name)                                       \
        static HMODULE DLL_HANDLE_NAME(name)
 
-#define DLL_GET_HANDLE(name)                                   \
-       do {                                                    \
-               DLL_HANDLE_NAME(name) = DLL_LOAD_LIBRARY(name); \
-               if (!DLL_HANDLE_NAME(name))                     \
-                       return false;                           \
+#define DLL_GET_HANDLE(ctx, name)                                      \
+       do {                                                            \
+               DLL_HANDLE_NAME(name) = load_system_library(ctx,        \
+                               DLL_STRINGIFY(name));                   \
+               if (!DLL_HANDLE_NAME(name))                             \
+                       return false;                                   \
        } while (0)
 
-#define DLL_FREE_HANDLE(name)                                  \
-       do {                                                    \
-               if (DLL_HANDLE_NAME(name)) {                    \
-                       FreeLibrary(DLL_HANDLE_NAME(name));     \
-                       DLL_HANDLE_NAME(name) = NULL;           \
-               }                                               \
+#define DLL_FREE_HANDLE(name)                                          \
+       do {                                                            \
+               if (DLL_HANDLE_NAME(name)) {                            \
+                       FreeLibrary(DLL_HANDLE_NAME(name));             \
+                       DLL_HANDLE_NAME(name) = NULL;                   \
+               }                                                       \
        } while (0)
 
 /*
@@ -377,6 +377,7 @@ static inline struct winusb_transfer_priv *get_winusb_transfer_priv(struct usbi_
 extern const struct windows_backend usbdk_backend;
 extern const struct windows_backend winusb_backend;
 
+HMODULE load_system_library(struct libusb_context *ctx, const char *name);
 unsigned long htab_hash(const char *str);
 enum libusb_transfer_status usbd_status_to_libusb_transfer_status(USBD_STATUS status);
 void windows_force_sync_completion(struct usbi_transfer *itransfer, ULONG size);

diff --git a/libusb/os/windows_usbdk.c b/libusb/os/windows_usbdk.c
index cdfbb17..c9ebfcf 100644 (file)
--- a/libusb/os/windows_usbdk.c
+++ b/libusb/os/windows_usbdk.c
@@ -81,7 +81,7 @@ static void unload_usbdk_helper_dll(void)
 
 static int load_usbdk_helper_dll(struct libusb_context *ctx)
 {
-       usbdk_helper.module = LoadLibraryA("UsbDkHelper");
+       usbdk_helper.module = load_system_library(ctx, "UsbDkHelper");
        if (usbdk_helper.module == NULL) {
                usbi_err(ctx, "Failed to load UsbDkHelper.dll: %s", windows_error_str(0));
                return LIBUSB_ERROR_NOT_FOUND;
@@ -160,7 +160,7 @@ static int usbdk_init(struct libusb_context *ctx)
        SC_HANDLE serviceHandle;
        HMODULE h;
 
-       h = LoadLibraryA("Advapi32");
+       h = load_system_library(ctx, "Advapi32");
        if (h == NULL) {
                usbi_warn(ctx, "failed to open Advapi32\n");
                return LIBUSB_ERROR_OTHER;

diff --git a/libusb/os/windows_winusb.c b/libusb/os/windows_winusb.c
index 393525f..0f857ed 100644 (file)
--- a/libusb/os/windows_winusb.c
+++ b/libusb/os/windows_winusb.c
@@ -146,21 +146,21 @@ static char *normalize_path(const char *path)
 /*
  * Cfgmgr32, AdvAPI32, OLE32 and SetupAPI DLL functions
  */
-static bool init_dlls(void)
+static bool init_dlls(struct libusb_context *ctx)
 {
-       DLL_GET_HANDLE(Cfgmgr32);
+       DLL_GET_HANDLE(ctx, Cfgmgr32);
        DLL_LOAD_FUNC(Cfgmgr32, CM_Get_Parent, true);
        DLL_LOAD_FUNC(Cfgmgr32, CM_Get_Child, true);
 
        // Prefixed to avoid conflict with header files
-       DLL_GET_HANDLE(AdvAPI32);
+       DLL_GET_HANDLE(ctx, AdvAPI32);
        DLL_LOAD_FUNC_PREFIXED(AdvAPI32, p, RegQueryValueExW, true);
        DLL_LOAD_FUNC_PREFIXED(AdvAPI32, p, RegCloseKey, true);
 
-       DLL_GET_HANDLE(OLE32);
+       DLL_GET_HANDLE(ctx, OLE32);
        DLL_LOAD_FUNC_PREFIXED(OLE32, p, IIDFromString, true);
 
-       DLL_GET_HANDLE(SetupAPI);
+       DLL_GET_HANDLE(ctx, SetupAPI);
        DLL_LOAD_FUNC_PREFIXED(SetupAPI, p, SetupDiGetClassDevsA, true);
        DLL_LOAD_FUNC_PREFIXED(SetupAPI, p, SetupDiEnumDeviceInfo, true);
        DLL_LOAD_FUNC_PREFIXED(SetupAPI, p, SetupDiEnumDeviceInterfaces, true);
@@ -643,7 +643,7 @@ static int winusb_init(struct libusb_context *ctx)
        int i;
 
        // Load DLL imports
-       if (!init_dlls()) {
+       if (!init_dlls(ctx)) {
                usbi_err(ctx, "could not resolve DLL functions");
                return LIBUSB_ERROR_OTHER;
        }
@@ -2192,7 +2192,7 @@ static bool winusbx_init(struct libusb_context *ctx)
 {
        HMODULE hWinUSB, hlibusbK;
 
-       hWinUSB = LoadLibraryA("WinUSB");
+       hWinUSB = load_system_library(ctx, "WinUSB");
        if (hWinUSB != NULL) {
                WinUSB_Set(hWinUSB, AbortPipe, true);
                WinUSB_Set(hWinUSB, ControlTransfer, true);
@@ -2231,7 +2231,7 @@ cleanup_winusb:
                usbi_info(ctx, "WinUSB DLL is not available");
        }
 
-       hlibusbK = LoadLibraryA("libusbK");
+       hlibusbK = load_system_library(ctx, "libusbK");
        if (hlibusbK != NULL) {
                LibK_GetVersion_t pLibK_GetVersion;
                LibK_GetProcAddress_t pLibK_GetProcAddress;
@@ -3592,9 +3592,7 @@ static int _hid_class_request(struct libusb_device *dev, HANDLE hid_handle, int
  */
 static bool hid_init(struct libusb_context *ctx)
 {
-       UNUSED(ctx);
-
-       DLL_GET_HANDLE(hid);
+       DLL_GET_HANDLE(ctx, hid);
 
        DLL_LOAD_FUNC(hid, HidD_GetAttributes, true);
        DLL_LOAD_FUNC(hid, HidD_GetHidGuid, true);

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index f4b4f9f..5d3763d 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11568
+#define LIBUSB_NANO 11569