diff mbox series

[Openvpn-devel,v2] Prevent crash on invalid server-ipv6 argument

Message ID 20251207210529.9949-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v2] Prevent crash on invalid server-ipv6 argument | expand

Commit Message

Gert Doering Dec. 7, 2025, 9:05 p.m. UTC 
From: Klemens Nanni <kn@openbsd.org>

`get_addr_generic()` expects `openvpn_getaddrinfo()` to return a newly
allocated struct, but getaddrinfo(3) failure leaves `*ai = NULL` as-is.

On OpenBSD, unlike free(3), freegetaddrinfo(3) requires a valid struct,
thus callers must check the argument to avoid NULL-deref or double-free:

```
$ openvpn --server-ipv6 ''
2025-12-06 11:59:18 RESOLVE: Cannot resolve host address: :[AF_INET6] (no address associated with name)
Segmentation fault (core dumped)
```

Guard against empty `ai`, i.e. failure, like similar code already does:

```
$ ./openvpn --server-ipv6 ''
2025-12-06 12:05:11 RESOLVE: Cannot resolve host address: :[AF_INET6] (no address associated with name)
Options error: error parsing --server-ipv6 parameter
Use --help for more information.
```

Spotted through a configuration typo "server-ipv6 fd00:/64" with 2.6.17,
reproduced with and tested against 2.7rc3 on OpenBSD/amd64 7.8-current.


NB: Standards are unclear wrt. freeaddrinfo(3)'s NULL handling;
    Linux, FreeBSD and illumos do check it and thus not crash.

Change-Id: I99a6604fdfc682f9609bfe7672aa78285084dcb9
Signed-off-by: Klemens Nanni <kn@openbsd.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1418
---

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

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

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Dec. 8, 2025, 11:33 a.m. UTC | #1 
Thanks for your patch.  

Repeating what was said in https://github.com/OpenVPN/openvpn/pull/930,
for the sake of the archives - on FreeBSD and Linux, this does not crash,
because the libraries handle "ai == NULL" gracefully.  The FreeBSD 
implementation mentions that the standard is not clear, so OpenBSD is
free to crash on us.

This said, we do guard all other paths to freeaddrinfo() (either because
the call is only on the "success" branch, or with an explicit check) - so
this is a good fix to make our code consistent.

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

commit 0ff66c056f951dcf01cf6ccb3e9b21948e5ca5ad (master)
commit 09c35f8421028ce8aed4895a526c2f4c4b4be01b (release/2.6)
Author: Klemens Nanni
Date:   Sun Dec 7 22:05:18 2025 +0100

     Prevent crash on invalid server-ipv6 argument

     Signed-off-by: Klemens Nanni <kn@openbsd.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1418
     Message-Id: <20251207210529.9949-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg34870.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 46bedf4..80c2895 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -189,7 +189,10 @@ 
         *sep = '/';
     }
 out:
-    freeaddrinfo(ai);
+    if (ai)
+    {
+        freeaddrinfo(ai);
+    }
     free(var_host);
 
     return ret;