cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1706?usp=email )
Change subject: socket: assert buffer length before reading prepended sockaddr family ...................................................................... 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 <[email protected]> Acked-by: Gert Doering <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1706 Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg37065.html Signed-off-by: Gert Doering <[email protected]> --- M src/openvpn/socket.c 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c index 624ce4f..df2cc9e 100644 --- a/src/openvpn/socket.c +++ b/src/openvpn/socket.c @@ -2813,38 +2813,31 @@ { 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; } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1706?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I2ce954aa5b74002be5e38d53783435736625bb2f Gerrit-Change-Number: 1706 Gerrit-PatchSet: 2 Gerrit-Owner: stipa <[email protected]> Gerrit-Reviewer: cron2 <[email protected]> Gerrit-Reviewer: plaisthos <[email protected]> Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
