Browse code

win32: Enforce loading of plugins from a trusted directory

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>

Lev Stipakov authored on 2024/03/19 22:53:45
Showing 3 changed files
... ...
@@ -277,11 +277,23 @@ plugin_init_item(struct plugin *p, const struct plugin_option *o)
277 277
 
278 278
 #else  /* ifndef _WIN32 */
279 279
 
280
-    rel = !platform_absolute_pathname(p->so_pathname);
281
-    p->module = LoadLibraryW(wide_string(p->so_pathname, &gc));
280
+    WCHAR *wpath = wide_string(p->so_pathname, &gc);
281
+    WCHAR normalized_plugin_path[MAX_PATH] = {0};
282
+    /* Normalize the plugin path, converting any relative paths to absolute paths. */
283
+    if (!GetFullPathNameW(wpath, MAX_PATH, normalized_plugin_path, NULL))
284
+    {
285
+        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %ls. Failed to normalize plugin path.", wpath);
286
+    }
287
+
288
+    if (!plugin_in_trusted_dir(normalized_plugin_path))
289
+    {
290
+        msg(M_FATAL, "PLUGIN_INIT: could not load plugin DLL: %ls. The DLL is not in a trusted directory.", normalized_plugin_path);
291
+    }
292
+
293
+    p->module = LoadLibraryW(normalized_plugin_path);
282 294
     if (!p->module)
283 295
     {
284
-        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %s", p->so_pathname);
296
+        msg(M_ERR, "PLUGIN_INIT: could not load plugin DLL: %ls", normalized_plugin_path);
285 297
     }
286 298
 
287 299
 #define PLUGIN_SYM(var, name, flags) dll_resolve_symbol(p->module, (void *)&p->var, name, p->so_pathname, flags)
... ...
@@ -1497,27 +1497,24 @@ openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const for
1497 1497
     return (len >= 0 && len < size);
1498 1498
 }
1499 1499
 
1500
-static BOOL
1501
-get_install_path(WCHAR *path, DWORD size)
1500
+bool
1501
+get_openvpn_reg_value(const WCHAR *key, WCHAR *value, DWORD size)
1502 1502
 {
1503 1503
     WCHAR reg_path[256];
1504
-    HKEY key;
1505
-    BOOL res = FALSE;
1504
+    HKEY hkey;
1506 1505
     openvpn_swprintf(reg_path, _countof(reg_path), L"SOFTWARE\\" PACKAGE_NAME);
1507 1506
 
1508
-    LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &key);
1507
+    LONG status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, reg_path, 0, KEY_READ, &hkey);
1509 1508
     if (status != ERROR_SUCCESS)
1510 1509
     {
1511
-        return res;
1510
+        return false;
1512 1511
     }
1513 1512
 
1514
-    /* The default value of REG_KEY is the install path */
1515
-    status = RegGetValueW(key, NULL, NULL, RRF_RT_REG_SZ, NULL, (LPBYTE)path, &size);
1516
-    res = status == ERROR_SUCCESS;
1513
+    status = RegGetValueW(hkey, NULL, key, RRF_RT_REG_SZ, NULL, (LPBYTE)value, &size);
1517 1514
 
1518
-    RegCloseKey(key);
1515
+    RegCloseKey(hkey);
1519 1516
 
1520
-    return res;
1517
+    return status == ERROR_SUCCESS;
1521 1518
 }
1522 1519
 
1523 1520
 static void
... ...
@@ -1526,7 +1523,7 @@ set_openssl_env_vars()
1526 1526
     const WCHAR *ssl_fallback_dir = L"C:\\Windows\\System32";
1527 1527
 
1528 1528
     WCHAR install_path[MAX_PATH] = { 0 };
1529
-    if (!get_install_path(install_path, _countof(install_path)))
1529
+    if (!get_openvpn_reg_value(NULL, install_path, _countof(install_path)))
1530 1530
     {
1531 1531
         /* if we cannot find installation path from the registry,
1532 1532
          * use Windows directory as a fallback
... ...
@@ -1605,4 +1602,60 @@ win32_sleep(const int n)
1605 1605
         }
1606 1606
     }
1607 1607
 }
1608
+
1609
+bool
1610
+plugin_in_trusted_dir(const WCHAR *plugin_path)
1611
+{
1612
+    /* UNC paths are not allowed */
1613
+    if (wcsncmp(plugin_path, L"\\\\", 2) == 0)
1614
+    {
1615
+        msg(M_WARN, "UNC paths for plugins are not allowed.");
1616
+        return false;
1617
+    }
1618
+
1619
+    WCHAR plugin_dir[MAX_PATH] = { 0 };
1620
+
1621
+    /* Attempt to retrieve the trusted plugin directory path from the registry,
1622
+     * using installation path as a fallback */
1623
+    if (!get_openvpn_reg_value(L"plugin_dir", plugin_dir, _countof(plugin_dir))
1624
+        && !get_openvpn_reg_value(NULL, plugin_dir, _countof(plugin_dir)))
1625
+    {
1626
+        msg(M_WARN, "Installation path could not be determined.");
1627
+    }
1628
+
1629
+    /* Get the system directory */
1630
+    WCHAR system_dir[MAX_PATH] = { 0 };
1631
+    if (GetSystemDirectoryW(system_dir, _countof(system_dir)) == 0)
1632
+    {
1633
+        msg(M_NONFATAL | M_ERRNO, "Failed to get system directory.");
1634
+    }
1635
+
1636
+    if ((wcslen(plugin_dir) == 0) && (wcslen(system_dir) == 0))
1637
+    {
1638
+        return false;
1639
+    }
1640
+
1641
+    WCHAR normalized_plugin_dir[MAX_PATH] = { 0 };
1642
+
1643
+    /* Normalize the plugin dir */
1644
+    if (wcslen(plugin_dir) > 0)
1645
+    {
1646
+        if (!GetFullPathNameW(plugin_dir, MAX_PATH, normalized_plugin_dir, NULL))
1647
+        {
1648
+            msg(M_NONFATAL | M_ERRNO, "Failed to normalize plugin dir.");
1649
+            return false;
1650
+        }
1651
+    }
1652
+
1653
+    /* Check if the plugin path resides within the plugin/install directory */
1654
+    if ((wcslen(normalized_plugin_dir) > 0) && (wcsnicmp(normalized_plugin_dir,
1655
+                                                         plugin_path, wcslen(normalized_plugin_dir)) == 0))
1656
+    {
1657
+        return true;
1658
+    }
1659
+
1660
+    /* Fallback to the system directory */
1661
+    return wcsnicmp(system_dir, plugin_path, wcslen(system_dir)) == 0;
1662
+}
1663
+
1608 1664
 #endif /* ifdef _WIN32 */
... ...
@@ -330,5 +330,32 @@ openvpn_swprintf(wchar_t *const str, const size_t size, const wchar_t *const for
330 330
 /* Sleep that can be interrupted by signals and exit event */
331 331
 void win32_sleep(const int n);
332 332
 
333
+/**
334
+ * @brief Fetches a registry value for OpenVPN registry key.
335
+ *
336
+ * @param key Registry value name to fetch.
337
+ * @param value Buffer to store the fetched string value.
338
+ * @param size Size of `value` buffer in bytes.
339
+ * @return `true` if successful, `false` otherwise.
340
+ */
341
+bool
342
+get_openvpn_reg_value(const WCHAR *key, WCHAR *value, DWORD size);
343
+
344
+/**
345
+ * @brief Checks if a plugin is located in a trusted directory.
346
+ *
347
+ * Verifies the plugin's path against a trusted directory, which is:
348
+ *
349
+ * - "plugin_dir" registry value or installation path, if the registry key is missing
350
+ * - system directory
351
+ *
352
+ * UNC paths are explicitly disallowed.
353
+ *
354
+ * @param plugin_path Normalized path to the plugin.
355
+ * @return \c true if the plugin is in a trusted directory and not a UNC path; \c false otherwise.
356
+ */
357
+bool
358
+plugin_in_trusted_dir(const WCHAR *plugin_path);
359
+
333 360
 #endif /* ifndef OPENVPN_WIN32_H */
334 361
 #endif /* ifdef _WIN32 */