From: Steven <sluong@cisco.com>
Date: Wed, 8 Nov 2017 22:14:45 +0000 (-0800)
Subject: VCL/LDPRELOAD: Fix out-of-bounds access and inequality comparison coverity errors
X-Git-Tag: v18.04-rc0~260
X-Git-Url: https://gerrit.fd.io/r/gitweb?p=vpp.git;a=commitdiff_plain;h=0cdd5bd05bbb78c28a6995eff1d80eeabb9d6fd1

VCL/LDPRELOAD: Fix out-of-bounds access and inequality comparison coverity errors

Fixed out-of-bounds access in vcom_socket.c by limiting the copy to the
size of the address field that was passed. Truncation will occur if
the address field is not big enough.

Fixed inequality comparison in vppcom.c by using the predefined macro
MAP_FAILED.

Change-Id: I9517c29ae811d08058621bd548a352b4d4f05139
Signed-off-by: Steven <sluong@cisco.com>
---

diff --git a/src/vcl/vcom_socket.c b/src/vcl/vcom_socket.c
index b2d6f58d794..a84359e8417 100644
--- a/src/vcl/vcom_socket.c
+++ b/src/vcl/vcom_socket.c
@@ -1413,6 +1413,47 @@ vcom_session_getpeername (int sid, vppcom_endpt_t * ep)
   return rv;
 }
 
+static inline int
+vcom_socket_copy_ep_to_sockaddr (__SOCKADDR_ARG __addr,
+				 socklen_t * __restrict __len,
+				 vppcom_endpt_t * ep)
+{
+  int rv = 0;
+  int sa_len, copy_len;
+
+  __addr->sa_family = (ep->is_ip4 == VPPCOM_IS_IP4) ? AF_INET : AF_INET6;
+  switch (__addr->sa_family)
+    {
+    case AF_INET:
+      ((struct sockaddr_in *) __addr)->sin_port = ep->port;
+      if (*__len > sizeof (struct sockaddr_in))
+	*__len = sizeof (struct sockaddr_in);
+      sa_len = sizeof (struct sockaddr_in) - sizeof (struct in_addr);
+      copy_len = *__len - sa_len;
+      if (copy_len > 0)
+	memcpy (&((struct sockaddr_in *) __addr)->sin_addr, ep->ip, copy_len);
+      break;
+
+    case AF_INET6:
+      ((struct sockaddr_in6 *) __addr)->sin6_port = ep->port;
+      if (*__len > sizeof (struct sockaddr_in6))
+	*__len = sizeof (struct sockaddr_in6);
+      sa_len = sizeof (struct sockaddr_in6) - sizeof (struct in6_addr);
+      copy_len = *__len - sa_len;
+      if (copy_len > 0)
+	memcpy (((struct sockaddr_in6 *) __addr)->sin6_addr.
+		__in6_u.__u6_addr8, ep->ip, copy_len);
+      break;
+
+    default:
+      /* Not possible */
+      rv = -EAFNOSUPPORT;
+      break;
+    }
+
+  return rv;
+}
+
 int
 vcom_socket_getpeername (int __fd, __SOCKADDR_ARG __addr,
 			 socklen_t * __restrict __len)
@@ -1421,7 +1462,8 @@ vcom_socket_getpeername (int __fd, __SOCKADDR_ARG __addr,
   vcom_socket_main_t *vsm = &vcom_socket_main;
   uword *p;
   vcom_socket_t *vsock;
-
+  u8 src_addr[sizeof (struct sockaddr_in6)];
+  vppcom_endpt_t ep;
 
   p = hash_get (vsm->sockidx_by_fd, __fd);
   if (!p)
@@ -1437,31 +1479,10 @@ vcom_socket_getpeername (int __fd, __SOCKADDR_ARG __addr,
   if (!__addr || !__len)
     return -EFAULT;
 
-  vppcom_endpt_t ep;
-  ep.ip = (u8 *) & ((const struct sockaddr_in *) __addr)->sin_addr;
+  ep.ip = src_addr;
   rv = vcom_session_getpeername (vsock->sid, &ep);
   if (rv == 0)
-    {
-      if (ep.vrf == VPPCOM_VRF_DEFAULT)
-	{
-	  __addr->sa_family = ep.is_ip4 == VPPCOM_IS_IP4 ? AF_INET : AF_INET6;
-	  switch (__addr->sa_family)
-	    {
-	    case AF_INET:
-	      ((struct sockaddr_in *) __addr)->sin_port = ep.port;
-	      *__len = sizeof (struct sockaddr_in);
-	      break;
-
-	    case AF_INET6:
-	      ((struct sockaddr_in6 *) __addr)->sin6_port = ep.port;
-	      *__len = sizeof (struct sockaddr_in6);
-	      break;
-
-	    default:
-	      break;
-	    }
-	}
-    }
+    rv = vcom_socket_copy_ep_to_sockaddr (__addr, __len, &ep);
 
   return rv;
 }
@@ -1633,37 +1654,7 @@ vcom_session_recvfrom (int __sid, void *__restrict __buf, size_t __n,
       rv = vppcom_session_recvfrom (__sid, __buf, __n, __flags, &ep);
 
       if (rv > 0)
-	{
-	  if (ep.vrf == VPPCOM_VRF_DEFAULT)
-	    {
-	      __addr->sa_family =
-		ep.is_ip4 == VPPCOM_IS_IP4 ? AF_INET : AF_INET6;
-	      switch (__addr->sa_family)
-		{
-		case AF_INET:
-		  ((struct sockaddr_in *) __addr)->sin_port = ep.port;
-		  memcpy (&((struct sockaddr_in *) __addr)->sin_addr,
-			  src_addr, sizeof (struct in_addr));
-
-		  *__addr_len = sizeof (struct sockaddr_in);
-		  break;
-
-		case AF_INET6:
-		  ((struct sockaddr_in6 *) __addr)->sin6_port = ep.port;
-		  memcpy (((struct sockaddr_in6 *) __addr)->sin6_addr.
-			  __in6_u.__u6_addr8, src_addr,
-			  sizeof (struct in6_addr));
-		  *__addr_len = sizeof (struct sockaddr_in6);
-		  break;
-
-		default:
-		  rv = -EAFNOSUPPORT;
-		  break;
-		}
-	    }
-	  else
-	    rv = -1;
-	}
+	rv = vcom_socket_copy_ep_to_sockaddr (__addr, __addr_len, &ep);
     }
   else
     rv = vppcom_session_recvfrom (__sid, __buf, __n, __flags, NULL);
diff --git a/src/vcl/vppcom.c b/src/vcl/vppcom.c
index 2382f29a2e1..a46bfe2d8db 100644
--- a/src/vcl/vppcom.c
+++ b/src/vcl/vppcom.c
@@ -1437,7 +1437,7 @@ defaulted:
 
   vcl_mem = mmap (0, vcl_cfg->heapsize, PROT_READ | PROT_WRITE,
 		  MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-  if (vcl_mem <= 0)
+  if (vcl_mem == MAP_FAILED)
     {
       clib_unix_error ("[%d] ERROR: mmap(0, %lld == 0x%llx, "
 		       "PROT_READ | PROT_WRITE,MAP_SHARED | MAP_ANONYMOUS, "