From 762aafec34478bcef01a16acf1959732ab8bb2b6 Mon Sep 17 00:00:00 2001
From: Florian Weimer <fweimer@redhat.com>
Date: Fri, 29 Apr 2016 10:35:34 +0200
Subject: [PATCH] CVE-2016-3706: getaddrinfo: stack overflow in hostent conversion [BZ #20010]

When converting a struct hostent response to struct gaih_addrtuple, the
gethosts macro (which is called from gaih_inet) used alloca, without
malloc fallback for large responses.  This commit changes this code to
use calloc unconditionally.

This commit also consolidated a second hostent-to-gaih_addrtuple
conversion loop (in gaih_inet) to use the new conversion function.

(cherry picked from commit 4ab2ab03d4351914ee53248dc5aef4a8c88ff8b9)
---
 ChangeLog                   |   10 +++
 NEWS                        |    7 ++-
 sysdeps/posix/getaddrinfo.c |  130 +++++++++++++++++++++++--------------------
 3 files changed, 85 insertions(+), 62 deletions(-)

#diff --git a/ChangeLog b/ChangeLog
#index c55bff5..671f2bf 100644
#--- a/ChangeLog
#+++ b/ChangeLog
#@@ -1,3 +1,13 @@
#+2016-04-29  Florian Weimer  <fweimer@redhat.com>
#+
#+	[BZ #20010]
#+	CVE-2016-3706
#+	* sysdeps/posix/getaddrinfo.c
#+	(convert_hostent_to_gaih_addrtuple): New function.
#+	(gethosts): Call convert_hostent_to_gaih_addrtuple.
#+	(gaih_inet): Use convert_hostent_to_gaih_addrtuple to convert
#+	AF_INET data.
#+
# 2016-05-04  Florian Weimer  <fweimer@redhat.com>
# 
# 	[BZ #19779]
#diff --git a/NEWS b/NEWS
#index 00a8add..5304a05 100644
#--- a/NEWS
#+++ b/NEWS
#@@ -12,7 +12,7 @@ Version 2.19.1
#   15946, 16545, 16574, 16623, 16657, 16695, 16743, 16758, 16759, 16760,
#   16878, 16882, 16885, 16916, 16932, 16943, 16958, 17048, 17062, 17069,
#   17079, 17137, 17153, 17213, 17263, 17269, 17325, 17555, 17905, 18007,
#-  18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879.
#+  18032, 18080, 18240, 18287, 18508, 18905, 19779, 19879, 20010.
# 
# * A buffer overflow in gethostbyname_r and related functions performing DNS
#   requests has been fixed.  If the NSS functions were called with a
#@@ -72,6 +72,11 @@ Version 2.19.1
# * The glob function suffered from a stack-based buffer overflow when it was
#   called with the GLOB_ALTDIRFUNC flag and encountered a long file name.
#   Reported by Alexander Cherepanov.  (CVE-2016-1234)
#+
#+* Previously, getaddrinfo copied large amounts of address data to the stack,
#+  even after the fix for CVE-2013-4458 has been applied, potentially
#+  resulting in a stack overflow.  getaddrinfo now uses a heap allocation
#+  instead.  Reported by Michael Petlan.  (CVE-2016-3706)
# 
# Version 2.19
# 
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index d2283bc..df6ce8b 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -168,9 +168,58 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
   return 0;
 }
 
+/* Convert struct hostent to a list of struct gaih_addrtuple objects.
+   h_name is not copied, and the struct hostent object must not be
+   deallocated prematurely.  *RESULT must be NULL or a pointer to an
+   object allocated using malloc, which is freed.  */
+static bool
+convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
+				   int family,
+				   struct hostent *h,
+				   struct gaih_addrtuple **result)
+{
+  free (*result);
+  *result = NULL;
+
+  /* Count the number of addresses in h->h_addr_list.  */
+  size_t count = 0;
+  for (char **p = h->h_addr_list; *p != NULL; ++p)
+    ++count;
+
+  /* Report no data if no addresses are available, or if the incoming
+     address size is larger than what we can store.  */
+  if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
+    return true;
+
+  struct gaih_addrtuple *array = calloc (count, sizeof (*array));
+  if (array == NULL)
+    return false;
+
+  for (size_t i = 0; i < count; ++i)
+    {
+      if (family == AF_INET && req->ai_family == AF_INET6)
+	{
+	  /* Perform address mapping. */
+	  array[i].family = AF_INET6;
+	  memcpy(array[i].addr + 3, h->h_addr_list[i], sizeof (uint32_t));
+	  array[i].addr[2] = htonl (0xffff);
+	}
+      else
+	{
+	  array[i].family = family;
+	  memcpy (array[i].addr, h->h_addr_list[i], h->h_length);
+	}
+      array[i].next = array + i + 1;
+    }
+  array[0].name = h->h_name;
+  array[count - 1].next = NULL;
+
+  *result = array;
+  return true;
+}
+
 #define gethosts(_family, _type) \
  {									      \
-  int i;								      \
   int herrno;								      \
   struct hostent th;							      \
   struct hostent *h;							      \
@@ -219,36 +268,23 @@ gaih_inet_serv (const char *servicename, const struct gaih_typeproto *tp,
     }									      \
   else if (h != NULL)							      \
     {									      \
-      for (i = 0; h->h_addr_list[i]; i++)				      \
+      /* Make sure that addrmem can be freed.  */			      \
+      if (!malloc_addrmem)						      \
+	addrmem = NULL;							      \
+      if (!convert_hostent_to_gaih_addrtuple (req, _family,h, &addrmem))      \
 	{								      \
-	  if (*pat == NULL)						      \
-	    {								      \
-	      *pat = __alloca (sizeof (struct gaih_addrtuple));		      \
-	      (*pat)->scopeid = 0;					      \
-	    }								      \
-	  uint32_t *addr = (*pat)->addr;				      \
-	  (*pat)->next = NULL;						      \
-	  (*pat)->name = i == 0 ? strdupa (h->h_name) : NULL;		      \
-	  if (_family == AF_INET && req->ai_family == AF_INET6)		      \
-	    {								      \
-	      (*pat)->family = AF_INET6;				      \
-	      addr[3] = *(uint32_t *) h->h_addr_list[i];		      \
-	      addr[2] = htonl (0xffff);					      \
-	      addr[1] = 0;						      \
-	      addr[0] = 0;						      \
-	    }								      \
-	  else								      \
-	    {								      \
-	      (*pat)->family = _family;					      \
-	      memcpy (addr, h->h_addr_list[i], sizeof(_type));		      \
-	    }								      \
-	  pat = &((*pat)->next);					      \
+	  _res.options |= old_res_options & RES_USE_INET6;		      \
+	  result = -EAI_SYSTEM;						      \
+	  goto free_and_return;						      \
 	}								      \
+      *pat = addrmem;							      \
+      /* The conversion uses malloc unconditionally.  */		      \
+      malloc_addrmem = true;						      \
 									      \
       if (localcanon !=	NULL && canon == NULL)				      \
 	canon = strdupa (localcanon);					      \
 									      \
-      if (_family == AF_INET6 && i > 0)					      \
+      if (_family == AF_INET6 && *pat != NULL)				      \
 	got_ipv6 = true;						      \
     }									      \
  }
@@ -612,44 +648,16 @@ gaih_inet (const char *name, const struct gaih_service *service,
 		{
 		  if (h != NULL)
 		    {
-		      int i;
-		      /* We found data, count the number of addresses.  */
-		      for (i = 0; h->h_addr_list[i]; ++i)
-			;
-		      if (i > 0 && *pat != NULL)
-			--i;
-
-		      if (__libc_use_alloca (alloca_used
-					     + i * sizeof (struct gaih_addrtuple)))
-			addrmem = alloca_account (i * sizeof (struct gaih_addrtuple),
-						  alloca_used);
-		      else
-			{
-			  addrmem = malloc (i
-					    * sizeof (struct gaih_addrtuple));
-			  if (addrmem == NULL)
-			    {
-			      result = -EAI_MEMORY;
-			      goto free_and_return;
-			    }
-			  malloc_addrmem = true;
-			}
-
-		      /* Now convert it into the list.  */
-		      struct gaih_addrtuple *addrfree = addrmem;
-		      for (i = 0; h->h_addr_list[i]; ++i)
+		      /* We found data, convert it.  */
+		      if (!convert_hostent_to_gaih_addrtuple
+			  (req, AF_INET, h, &addrmem))
 			{
-			  if (*pat == NULL)
-			    {
-			      *pat = addrfree++;
-			      (*pat)->scopeid = 0;
-			    }
-			  (*pat)->next = NULL;
-			  (*pat)->family = AF_INET;
-			  memcpy ((*pat)->addr, h->h_addr_list[i],
-				  h->h_length);
-			  pat = &((*pat)->next);
+			  result = -EAI_MEMORY;
+			  goto free_and_return;
 			}
+		      *pat = addrmem;
+		      /* The conversion uses malloc unconditionally.  */
+		      malloc_addrmem = true;
 		    }
 		}
 	      else
-- 
1.7.1