Fix a use-after-free bug in get_recent_address() and two related issues.

1) The sockaddr_t * returned may be part of memory freed by the call to
   freeaddrinfo().
2) The sockaddr_t * returned from a recently seen address not in the
   cache was cast from struct addrinfo *ai, not the struct sockaddr *
   inside of it.
3) In do_outgoing_connection(), when filling in the address in the
   connection_t, there is a buffer overflow (read, not write) if
   the sa returned by get_recent_address() didn't come from the
   cache of recently seen addresses.  That is, it was really a
   struct sockaddr * and not a sockaddr_t *.  This last was
   found by building tinc with address sanitizer.

diff --git a/src/address_cache.c b/src/address_cache.c
index 42b671b8990208c5d402f9f6005c1450cbc7b19e..ff5850c97a8de38088029ec2c4e7004cd5c0ab17 100644 (file)
--- a/src/address_cache.c
+++ b/src/address_cache.c
@@ -126,7 +126,7 @@ const sockaddr_t *get_recent_address(address_cache_t *cache) {
 
                if(cache->ai) {
                        if(cache->aip) {
-                               sockaddr_t *sa = (sockaddr_t *)cache->aip;
+                               sockaddr_t *sa = (sockaddr_t *)cache->aip->ai_addr;
 
                                if(find_cached(cache, sa) != NOT_CACHED) {
                                        continue;
@@ -173,16 +173,16 @@ const sockaddr_t *get_recent_address(address_cache_t *cache) {
                cache->cfg = lookup_config_next(cache->config_tree, cache->cfg);
        }
 
-       if(cache->aip) {
-               sockaddr_t *sa = (sockaddr_t *)cache->aip->ai_addr;
-               cache->aip = cache->aip->ai_next;
+       if(cache->ai) {
+               if(cache->aip) {
+                       sockaddr_t *sa = (sockaddr_t *)cache->aip->ai_addr;
 
-               if(!cache->aip) {
+                       cache->aip = cache->aip->ai_next;
+                       return sa;
+               } else {
                        freeaddrinfo(cache->ai);
-                       cache->ai = cache->aip = NULL;
+                       cache->ai = NULL;
                }
-
-               return sa;
        }
 
        // We're all out of addresses.

diff --git a/src/net.h b/src/net.h
index 827194e7a802ea658a01dc5d0ad44b0800bc8feb..90cbdbb46188dec322a32c57dba7d174e57ae99f 100644 (file)
--- a/src/net.h
+++ b/src/net.h
@@ -74,9 +74,6 @@ typedef union sockaddr_t {
        struct sockaddr_in in;
        struct sockaddr_in6 in6;
        struct sockaddr_unknown unknown;
-#ifdef HAVE_STRUCT_SOCKADDR_STORAGE
-       struct sockaddr_storage storage;
-#endif
 } sockaddr_t;
 
 #ifdef SA_LEN

diff --git a/src/net_socket.c b/src/net_socket.c
index 30ab79e2b98ffdfa5f1eb7cd3bb12b7775a95af7..4fbcf57ddaa30e0d6f03690425c4d15ed9697bc4 100644 (file)
--- a/src/net_socket.c
+++ b/src/net_socket.c
@@ -59,14 +59,14 @@ static void configure_tcp(connection_t *c) {
        int flags = fcntl(c->socket, F_GETFL);
 
        if(fcntl(c->socket, F_SETFL, flags | O_NONBLOCK) < 0) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "fcntl for %s: %s", c->hostname, strerror(errno));
+               logger(DEBUG_ALWAYS, LOG_ERR, "fcntl for %s fd %d: %s", c->hostname, c->socket, strerror(errno));
        }
 
 #elif defined(WIN32)
        unsigned long arg = 1;
 
        if(ioctlsocket(c->socket, FIONBIO, &arg) != 0) {
-               logger(DEBUG_ALWAYS, LOG_ERR, "ioctlsocket for %s: %s", c->hostname, sockstrerror(sockerrno));
+               logger(DEBUG_ALWAYS, LOG_ERR, "ioctlsocket for %s fd %d: %s", c->hostname, c->socket, sockstrerror(sockerrno));
        }
 
 #endif
@@ -508,7 +508,7 @@ begin:
 
        connection_t *c = new_connection();
        c->outgoing = outgoing;
-       c->address = *sa;
+       memcpy(&c->address, sa, SALEN(sa->sa));
        c->hostname = sockaddr2hostname(&c->address);
 
        logger(DEBUG_CONNECTIONS, LOG_INFO, "Trying to connect to %s (%s)", outgoing->node->name, c->hostname);