From: Todd C. Miller <Todd.Miller@sudo.ws>
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);