Fix possible heap overflow on read accessing getaddrinfo() result.

The code always tried to copy-out a "struct sockaddr_in6" even for IPv4
results, which reads more bytes than getaddrinfo() is guaranteed to
allocate.

Now, look at ai->ai_family and only copy "struct sockaddr" for IPv4.

Also, reformat this block of code to comply to coding style.

This is a specific 2.3 bug as the code in master (to be 2.4) has been
completely rewritten to properly handle dual-stack and multiple responses
from getaddrinfo() proper.

Bug found by Daniel Hirche using "gcc -fsanitize=address".  No possible
exploits are known.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1447192719-31381-1-git-send-email-gert@greenie.muc.de>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10479

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index a143853e1239d8e6426f8edd81636a090036d70a..0f46bad6988cfc4de3e12bdcee4a95f8e5ca2aa9 100644 (file)
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1259,20 +1259,24 @@ resolve_remote (struct link_socket *sock,
                  ASSERT (0);
                }
 
-                 /* Temporary fix, this need to be changed for dual stack */
-                 status = openvpn_getaddrinfo(flags, sock->remote_host, retry,
-                                                                                         signal_received, af, &ai);
-                 if(status == 0) {
-                         sock->info.lsa->remote.addr.in6 = *((struct sockaddr_in6*)(ai->ai_addr));
-                         freeaddrinfo(ai);
+             /* Temporary fix, this need to be changed for dual stack */
+             status = openvpn_getaddrinfo(flags, sock->remote_host, retry,
+                                           signal_received, af, &ai);
+             if(status == 0)
+               {
+                 if ( ai->ai_family == AF_INET6 )
+                   sock->info.lsa->remote.addr.in6 = *((struct sockaddr_in6*)(ai->ai_addr));
+                 else
+                   sock->info.lsa->remote.addr.in4 = *((struct sockaddr_in*)(ai->ai_addr));
+                 freeaddrinfo(ai);
 
-                         dmsg (D_SOCKET_DEBUG, "RESOLVE_REMOTE flags=0x%04x phase=%d rrs=%d sig=%d status=%d",
+                 dmsg (D_SOCKET_DEBUG, "RESOLVE_REMOTE flags=0x%04x phase=%d rrs=%d sig=%d status=%d",
                                        flags,
                                        phase,
                                        retry,
                                        signal_received ? *signal_received : -1,
                                        status);
-                 }
+               }
              if (signal_received)
                {
                  if (*signal_received)