From: Lev Stipakov <[email protected]> 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 --- 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/+/1706 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <[email protected]> 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; } _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
