From dc0cbd0b68aafe4c3d71f19b3698dfa665de8d2f Mon Sep 17 00:00:00 2001
From: Stefan Berger <stefanb@us.ibm.com>
Date: Thu, 2 Apr 2020 14:42:06 +0000
Subject: [PATCH] [LibOS] Rework inet_copy_addr to avoid stack smashing when
 using IPv6

Rework inet_copy_addr by using sockaddr_storage (ported from glibc's
bits/socket.h) for writing the data into, which is large enough to hold
sockaddr_in6. Then copy back into the user's buffer.

Previously some calls to inet_copy_addr were passing in a
'struct sockaddr' that is too small to hold sockaddr_in6 data, thus
causing stack corruption when the socket was an IPv6 socket.
---
 LibOS/shim/include/shim_types.h  |  8 ++++
 LibOS/shim/src/sys/shim_socket.c | 67 ++++++++++++++------------------
 2 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/LibOS/shim/include/shim_types.h b/LibOS/shim/include/shim_types.h
index 046dfd6d..0ab7a531 100644
--- a/LibOS/shim/include/shim_types.h
+++ b/LibOS/shim/include/shim_types.h
@@ -379,6 +379,14 @@ struct sockaddr {
     char sa_data[14];           /* Address data.  */
 };
 
+/* From bits/socket.h */
+/* Structure large enough to hold any socket address (with the historical
+   exception of AF_UNIX).  */
+struct sockaddr_storage {
+    __SOCKADDR_COMMON(ss_);    /* Address family, etc.  */
+    char __ss_padding[128 - sizeof(sa_family_t)];
+};
+
 /* linux/mqueue.h */
 struct __kernel_mq_attr {
     long    mq_flags;       /* message queue flags */
diff --git a/LibOS/shim/src/sys/shim_socket.c b/LibOS/shim/src/sys/shim_socket.c
index 6aac89c9..9dfd5b44 100644
--- a/LibOS/shim/src/sys/shim_socket.c
+++ b/LibOS/shim/src/sys/shim_socket.c
@@ -329,24 +329,39 @@ static int inet_check_addr(int domain, struct sockaddr* addr, socklen_t addrlen)
     return -EINVAL;
 }
 
-static int inet_copy_addr(int domain, struct sockaddr* saddr, const struct addr_inet* addr) {
-    if (domain == AF_INET) {
-        struct sockaddr_in* in = (struct sockaddr_in*)saddr;
+static socklen_t inet_copy_addr(int domain, struct sockaddr* saddr, socklen_t saddr_len,
+                                const struct addr_inet* addr) {
+    struct sockaddr_storage ss;
+    struct sockaddr_in* in;
+    struct sockaddr_in6* in6;
+    socklen_t len = 0;
+
+    switch (domain) {
+      case AF_INET:
+        in = (struct sockaddr_in*)&ss;
         in->sin_family         = AF_INET;
         in->sin_port           = __htons(addr->port);
         in->sin_addr           = addr->addr.v4;
-        return sizeof(struct sockaddr_in);
-    }
 
-    if (domain == AF_INET6) {
-        struct sockaddr_in6* in6 = (struct sockaddr_in6*)saddr;
+        len = MIN(saddr_len, sizeof(struct sockaddr_in));
+        break;
+
+      case AF_INET6:
+        in6 = (struct sockaddr_in6*)&ss;
         in6->sin6_family         = AF_INET6;
         in6->sin6_port           = __htons(addr->port);
         in6->sin6_addr           = addr->addr.v6;
-        return sizeof(struct sockaddr_in6);
+
+        len = MIN(saddr_len, sizeof(struct sockaddr_in6));
+        break;
+
+      default:
+        __abort(); /* this function must accept only AF_INET/AF_INET6 */
     }
 
-    return sizeof(struct sockaddr);
+    memcpy(saddr, &ss, len);
+
+    return len;
 }
 
 static void inet_save_addr(int domain, struct addr_inet* addr, const struct sockaddr* saddr) {
@@ -943,14 +958,8 @@ int __do_accept(struct shim_handle* hdl, int flags, struct sockaddr* addr, sockl
         inet_rebase_port(true, cli_sock->domain, &cli_sock->addr.in.bind, true);
         inet_rebase_port(true, cli_sock->domain, &cli_sock->addr.in.conn, false);
 
-        if (addr) {
-            inet_copy_addr(sock->domain, addr, &sock->addr.in.conn);
-
-            if (addrlen) {
-                assert(sock->domain == AF_INET || sock->domain == AF_INET6);
-                *addrlen = minimal_addrlen(sock->domain);
-            }
-        }
+        if (addr)
+            *addrlen = inet_copy_addr(sock->domain, addr, *addrlen, &sock->addr.in.conn);
     }
 
     ret = set_new_fd_handle(cli, flags & O_CLOEXEC ? FD_CLOEXEC : 0, NULL);
@@ -1305,13 +1314,10 @@ static ssize_t do_recvmsg(int fd, struct iovec* bufs, int nbufs, int flags, stru
                     debug("last packet received from %s\n", uri);
 
                     inet_rebase_port(true, sock->domain, &conn, false);
-                    inet_copy_addr(sock->domain, addr, &conn);
+                    *addrlen = inet_copy_addr(sock->domain, addr, *addrlen, &conn);
                 } else {
-                    inet_copy_addr(sock->domain, addr, &sock->addr.in.conn);
+                    *addrlen = inet_copy_addr(sock->domain, addr, *addrlen, &sock->addr.in.conn);
                 }
-
-                *addrlen = (sock->domain == AF_INET) ? sizeof(struct sockaddr_in)
-                                                     : sizeof(struct sockaddr_in6);
             }
 
             address_received = true;
@@ -1486,14 +1492,8 @@ int shim_do_getsockname(int sockfd, struct sockaddr* addr, int* addrlen) {
     struct shim_sock_handle* sock = &hdl->info.sock;
     lock(&hdl->lock);
 
-    struct sockaddr saddr;
-    int len = inet_copy_addr(sock->domain, &saddr, &sock->addr.in.bind);
+    *addrlen = inet_copy_addr(sock->domain, addr, *addrlen, &sock->addr.in.bind);
 
-    if (len < *addrlen)
-        len = *addrlen;
-
-    memcpy(addr, &saddr, len);
-    *addrlen = len;
     ret      = 0;
     unlock(&hdl->lock);
 out:
@@ -1538,14 +1538,7 @@ int shim_do_getpeername(int sockfd, struct sockaddr* addr, int* addrlen) {
         goto out_locked;
     }
 
-    struct sockaddr saddr;
-    int len = inet_copy_addr(sock->domain, &saddr, &sock->addr.in.conn);
-
-    if (len < *addrlen)
-        len = *addrlen;
-
-    memcpy(addr, &saddr, len);
-    *addrlen = len;
+    *addrlen = inet_copy_addr(sock->domain, addr, *addrlen, &sock->addr.in.conn);
     ret      = 0;
 out_locked:
     unlock(&hdl->lock);