From: Lev Stipakov Date: Fri, 5 Jun 2026 14:18:02 +0000 (+0200) Subject: socket: assert buffer length before reading prepended sockaddr family X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=HEAD;p=thirdparty%2Fopenvpn.git socket: assert buffer length before reading prepended sockaddr family read_sockaddr_from_packet() inspected sa->sa_family before any check on buf->len, so a short delivery from the dco-win driver would have produced a garbage peer address from uninitialized buffer memory. The driver always prepends a full sockaddr and validates the family before writing, so reaching any of the size/family checks would mean something is severely wrong on the driver side - assert the three preconditions instead of M_FATAL'ing on them. GitHub: https://github.com/OpenVPN/openvpn-private-issues/issues/105 Change-Id: I2ce954aa5b74002be5e38d53783435736625bb2f Signed-off-by: Lev Stipakov Acked-by: Gert Doering Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1706 Message-Id: <20260605141808.14028-1-gert@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg37065.html Signed-off-by: Gert Doering --- diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 624ce4f53..df2cc9e03 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2813,38 +2813,31 @@ read_sockaddr_from_packet(struct buffer *buf, struct sockaddr *dst) { int sa_len = 0; + /* In dco-win multipeer mode the kernel driver always prepends a full + * sockaddr_in or sockaddr_in6 in front of the control-packet payload, + * so the buffer must hold at least sizeof(struct sockaddr_in) bytes + * before we may inspect sa_family. */ + ASSERT(buf_len(buf) >= (int)sizeof(struct sockaddr_in)); + const struct sockaddr *sa = (const struct sockaddr *)BPTR(buf); switch (sa->sa_family) { case AF_INET: sa_len = sizeof(struct sockaddr_in); - if (buf_len(buf) < sa_len) - { - msg(M_FATAL, - "ERROR: received incoming packet with too short length of %d -- must be at least %d.", - buf_len(buf), sa_len); - } - memcpy(dst, sa, sa_len); - buf_advance(buf, sa_len); break; case AF_INET6: sa_len = sizeof(struct sockaddr_in6); - if (buf_len(buf) < sa_len) - { - msg(M_FATAL, - "ERROR: received incoming packet with too short length of %d -- must be at least %d.", - buf_len(buf), sa_len); - } - memcpy(dst, sa, sa_len); - buf_advance(buf, sa_len); + ASSERT(buf_len(buf) >= sa_len); break; default: - msg(M_FATAL, "ERROR: received incoming packet with invalid address family %d.", - sa->sa_family); + ASSERT(0); /* driver validates the family before writing */ } + memcpy(dst, sa, sa_len); + buf_advance(buf, sa_len); + return sa_len; }