[S] Change in openvpn[master]: socket: assert buffer length before reading prepended sockaddr family

cron2 has uploaded a new patch set (#2) to the change originally created by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1706?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


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(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/06/1706/2

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: newpatchset
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