diff mbox series

[Openvpn-devel,v1] openvpnserv: detect sibling dirs in CheckConfigPath

Message ID 20260830182134.32251-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v1] openvpnserv: detect sibling dirs in CheckConfigPath |

Commit Message

Gert Doering Aug. 30, 2026, 6:21 p.m. UTC 
  From: Heiko Hund <heiko@ist.eigentlich.net>

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 <harshitvaru666@gmail.com>
Tested-By: Harshit Varu <harshitvaru666@gmail.com>
CVE: 2026-81830
Github: OpenVPN/openvpn-private-issues#166
Change-Id: Ica5d43989b441d4377a3908f811a2953b7a9d45a
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
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 <razvanc@mailbox.org>

Comments

Gert Doering Aug. 31, 2026, 6:47 a.m. UTC | #1 
This is, basically, a simplified backport of the fix that went into
"what became 2.7.0" - we did not do 2.6 at the time because the 2.7 fix
used a newer windows API that would have broken in-train compat for
2.6 (and then we forgot about it).

Arne's ACK was reported in the private GH repo, Razvan's both in Gerrit
and GH.  Harshit Varu reported this to us, and confirmed the patch fixes
the issue.  Thanks :-)

Your patch has been applied to the release/2.6 branch.

commit 927b18dff00a8ec0e9233a9689569e4519fc7de6 (release/2.6)
Author: Heiko Hund
Date:   Sun Aug 30 20:21:27 2026 +0200

     openvpnserv: detect sibling dirs in CheckConfigPath

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1883
     Message-Id: <20260830182134.32251-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg38828.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

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;
     }