From: Todd C. Miller Date: Fri, 16 Feb 2018 21:17:39 +0000 (-0700) Subject: Fix a use-after-free bug in get_recent_address() and two related issues. X-Git-Tag: release-1.1pre16~27 X-Git-Url: https://git.tinc-vpn.org/git/browse?a=commitdiff_plain;h=536fe3ffcdc4c894ed986dfb5fdc0d6f78d6fe25;p=tinc 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 42b671b8..ff5850c9 100644 --- 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 827194e7..90cbdbb4 100644 --- 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 30ab79e2..4fbcf57d 100644 --- 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);