]> git.ipfire.org Git - thirdparty/openvpn.git/commitdiff
win32: Enforce loading of plugins from a trusted directory
authorLev Stipakov <lstipakov@gmail.com>
Tue, 19 Mar 2024 13:53:45 +0000 (15:53 +0200)
committerGert Doering <gert@greenie.muc.de>
Tue, 19 Mar 2024 15:32:11 +0000 (16:32 +0100)
Currently, there's a risk associated with allowing plugins to be loaded
from any location. This update ensures plugins are only loaded from a
trusted directory, which is either:

    - HKLM\SOFTWARE\OpenVPN\plugin_dir (or if the key is missing,
    then HKLM\SOFTWARE\OpenVPN, which is installation directory)

    - System directory

Loading from UNC paths is disallowed.

Note: This change affects only Windows environments.

CVE: 2024-27903

Change-Id: I154a4aaad9242c9253a64312a14c5fd2ea95f40d
Reported-by: Vladimir Tokarev <vtokarev@microsoft.com>
Signed-off-by: Lev Stipakov <lev@openvpn.net>
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20240319135355.1279-2-lev@openvpn.net>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28416.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit aaea545d8a940f761898d736b68bcb067d503b1d)

src/openvpn/plugin.c
src/openvpn/win32.c
src/openvpn/win32.h

index ed5d7c06715e8e3cf6853ed387d8532c4de7df42..f7315f414b4d736b4662a8cc5c4a30a5dad74cea 100644 (file)
@@ -279,11 +279,23 @@ plugin_init_item(struct plugin *p, const struct plugin_option *o)
 
 #else  /* ifndef _WIN32 */
 
-    rel = !platform_absolute_pathname(p->so_pathname);
-    p->module = LoadLibraryW(wide_string(p->so_pathname, &gc));
+    WCHAR *wpath = wide_string(p->so_pathname, &gc);
+    WCHAR normalized_plugin_path[MAX_PATH] = {0};
+    /* Normalize the plugin path, converting any relative paths to absolute paths. */
+    if (!GetFullPathNameW(wpath, MAX_PATH, normalized_plugin_path, NULL))
+    {
+        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %ls. Failed to normalize plugin path.", wpath);
+    }
+
+    if (!plugin_in_trusted_dir(normalized_plugin_path))
+    {
+        msg(M_FATAL, "PLUGIN_INIT: could not load plugin DLL: %ls. The DLL is not in a trusted directory.", normalized_plugin_path);
+    }
+
+    p->module = LoadLibraryW(normalized_plugin_path);
     if (!p->module)
     {
-        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %s", p->so_pathname);
+        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %ls", normalized_plugin_path);
     }
 
 #define PLUGIN_SYM(var, name, flags) dll_resolve_symbol(p->module, (void *)&p->var, name, p->so_pathname, flags)
index e91e742bf2d33ef9406eaa9bdad2fd48adf96347..54aad8db12b3d56ba58e7a4d976029b5d00e599e 100644 (file)
@@ -1532,27 +1532,24 @@ openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const for
     return (len >= 0 && len < size);
 }
 
-static BOOL
-get_install_path(WCHAR *path, DWORD size)
+bool
+get_openvpn_reg_value(const WCHAR *key, WCHAR *value, DWORD size)
 {
     WCHAR reg_path[256];
-    HKEY key;
-    BOOL res = FALSE;
+    HKEY hkey;
     openvpn_swprintf(reg_path, _countof(reg_path), L"SOFTWARE\\" PACKAGE_NAME);
 
-    LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &key);
+    LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &hkey);
     if (status != ERROR_SUCCESS)
     {
-        return res;
+        return false;
     }
 
-    /* The default value of REG_KEY is the install path */
-    status = RegGetValueW(key, NULL, NULL, RRF_RT_REG_SZ, NULL, (LPBYTE)path, &size);
-    res = status == ERROR_SUCCESS;
+    status = RegGetValueW(hkey, NULL, key, RRF_RT_REG_SZ, NULL, (LPBYTE)value, &size);
 
-    RegCloseKey(key);
+    RegCloseKey(hkey);
 
-    return res;
+    return status == ERROR_SUCCESS;
 }
 
 static void
@@ -1561,7 +1558,7 @@ set_openssl_env_vars()
     const WCHAR *ssl_fallback_dir = L"C:\\Windows\\System32";
 
     WCHAR install_path[MAX_PATH] = { 0 };
-    if (!get_install_path(install_path, _countof(install_path)))
+    if (!get_openvpn_reg_value(NULL, install_path, _countof(install_path)))
     {
         /* if we cannot find installation path from the registry,
          * use Windows directory as a fallback
@@ -1597,4 +1594,59 @@ set_openssl_env_vars()
     }
 }
 
+bool
+plugin_in_trusted_dir(const WCHAR *plugin_path)
+{
+    /* UNC paths are not allowed */
+    if (wcsncmp(plugin_path, L"\\\\", 2) == 0)
+    {
+        msg(M_WARN, "UNC paths for plugins are not allowed.");
+        return false;
+    }
+
+    WCHAR plugin_dir[MAX_PATH] = { 0 };
+
+    /* Attempt to retrieve the trusted plugin directory path from the registry,
+     * using installation path as a fallback */
+    if (!get_openvpn_reg_value(L"plugin_dir", plugin_dir, _countof(plugin_dir))
+        && !get_openvpn_reg_value(NULL, plugin_dir, _countof(plugin_dir)))
+    {
+        msg(M_WARN, "Installation path could not be determined.");
+    }
+
+    /* Get the system directory */
+    WCHAR system_dir[MAX_PATH] = { 0 };
+    if (GetSystemDirectoryW(system_dir, _countof(system_dir)) == 0)
+    {
+        msg(M_NONFATAL | M_ERRNO, "Failed to get system directory.");
+    }
+
+    if ((wcslen(plugin_dir) == 0) && (wcslen(system_dir) == 0))
+    {
+        return false;
+    }
+
+    WCHAR normalized_plugin_dir[MAX_PATH] = { 0 };
+
+    /* Normalize the plugin dir */
+    if (wcslen(plugin_dir) > 0)
+    {
+        if (!GetFullPathNameW(plugin_dir, MAX_PATH, normalized_plugin_dir, NULL))
+        {
+            msg(M_NONFATAL | M_ERRNO, "Failed to normalize plugin dir.");
+            return false;
+        }
+    }
+
+    /* Check if the plugin path resides within the plugin/install directory */
+    if ((wcslen(normalized_plugin_dir) > 0) && (wcsnicmp(normalized_plugin_dir,
+                                                         plugin_path, wcslen(normalized_plugin_dir)) == 0))
+    {
+        return true;
+    }
+
+    /* Fallback to the system directory */
+    return wcsnicmp(system_dir, plugin_path, wcslen(system_dir)) == 0;
+}
+
 #endif /* ifdef _WIN32 */
index 3dd8b7e385d03000359039fbadb1f9e46834d671..e630f331e1ebe4f2a7e1181c9ed2892776f7b260 100644 (file)
@@ -335,5 +335,32 @@ openvpn_execve(const struct argv *a, const struct env_set *es, const unsigned in
 bool
 openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const format, ...);
 
+/**
+ * @brief Fetches a registry value for OpenVPN registry key.
+ *
+ * @param key Registry value name to fetch.
+ * @param value Buffer to store the fetched string value.
+ * @param size Size of `value` buffer in bytes.
+ * @return `true` if successful, `false` otherwise.
+ */
+bool
+get_openvpn_reg_value(const WCHAR *key, WCHAR *value, DWORD size);
+
+/**
+ * @brief Checks if a plugin is located in a trusted directory.
+ *
+ * Verifies the plugin's path against a trusted directory, which is:
+ *
+ * - "plugin_dir" registry value or installation path, if the registry key is missing
+ * - system directory
+ *
+ * UNC paths are explicitly disallowed.
+ *
+ * @param plugin_path Normalized path to the plugin.
+ * @return \c true if the plugin is in a trusted directory and not a UNC path; \c false otherwise.
+ */
+bool
+plugin_in_trusted_dir(const WCHAR *plugin_path);
+
 #endif /* ifndef OPENVPN_WIN32_H */
 #endif /* ifdef _WIN32 */