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 c72c4d266633f9749f1b2b6cd8758727d2eba126..f91075abd716978cfc7a42edaccec2c106e74a99 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 9f88956424e309f4badf7d80bc55180e5e2165f4..0c4b94c8444b0ee5e346303ecaf8b03c216e8108 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 cdfbb17108bbbb86bc34358508478a716c97527f..c9ebfcf79ba498f8f628d45042052214b2caa968 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 393525fc2f8280715f7a4e1573cf3379d8a196bf..0f857ed58bf7c296a3771029df7575063a80b1ee 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 f4b4f9fe465f28452d1b6d95db91e44acb1c4cdb..5d3763d937a1f46342d1379aeecf3b524a2c147f 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11568
+#define LIBUSB_NANO 11569