[PATCH v1] openvpnserv: detect sibling dirs in CheckConfigPath

Gert Doering <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <[email protected]>
From: Heiko Hund <[email protected]>

If the config_dir value in the registry has no trailing backslash the
check doesn't actually guarantee that a file is located within
config_dir, because a sibling dir with the same prefix, e.g. 'config'
and 'config-evil' will match and produce a positive verdict.

By also checking that there is a path separator after config_dir
prevents this attack.

Reported-By: Harshit Varu <[email protected]>
Tested-By: Harshit Varu <[email protected]>
CVE: 2026-81830
Github: OpenVPN/openvpn-private-issues#166
Change-Id: Ica5d43989b441d4377a3908f811a2953b7a9d45a
Signed-off-by: Heiko Hund <[email protected]>
Acked-by: Razvan Cojocaru <[email protected]>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1883
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to release/2.6.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1883
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <[email protected]>

        
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 770a7a0..b0fb6b85 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -56,7 +56,9 @@
 
 /*
  * Check workdir\fname is inside config_dir
- * The logic here is simple: we may reject some valid paths if ..\ is in any of the strings
+ * The logic here is simple:
+ *      we may reject some valid paths if ".." is in the filename
+ *      or if there's no "\" after the config directory
  */
 static BOOL
 CheckConfigPath(const WCHAR *workdir, const WCHAR *fname, const settings_t *s)
@@ -82,9 +84,18 @@
     }
 
     config_dir = s->config_dir;
+    size_t config_dir_len = wcslen(config_dir);
 
-    if (wcsncmp(config_dir, config_file, wcslen(config_dir)) == 0
-        && wcsstr(config_file + wcslen(config_dir), L"..") == NULL)
+    /* check for a path separator after config_dir */
+    if (config_dir_len && config_dir_len < wcslen(config_file)
+        && config_dir[config_dir_len - 1] != L'\\'
+        && config_file[config_dir_len] != L'\\')
+    {
+        return FALSE;
+    }
+
+    if (wcsncmp(config_dir, config_file, config_dir_len) == 0
+        && wcsstr(config_file + config_dir_len, L"..") == NULL)
     {
         return TRUE;
     }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.