[glibc/google/grte/v5-2.27/master] CVE-2016-10739: getaddrinfo: Fully parse IPv4 address strings [BZ #20018]

[Pranav Kant <pranavk@sourceware.org>]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=66bec53f073ca8ef6ac138dfe22ae9af7ce1d9db

commit 66bec53f073ca8ef6ac138dfe22ae9af7ce1d9db
Author: Florian Weimer <fweimer@redhat.com>
Date:   Mon Jan 21 21:26:03 2019 +0100

    CVE-2016-10739: getaddrinfo: Fully parse IPv4 address strings [BZ #20018]
    
    Some tests in original commit are not included because they depend on headers that
    are not present in GRTEv5 branch.
    
    The IPv4 address parser in the getaddrinfo function is changed so that
    it does not ignore trailing whitespace and all characters after it.
    For backwards compatibility, the getaddrinfo function still recognizes
    legacy name syntax, such as 192.000.002.010 interpreted as 192.0.2.8
    (octal).
    
    This commit does not change the behavior of inet_addr and inet_aton.
    gethostbyname already had additional sanity checks (but is switched
    over to the new __inet_aton_exact function for completeness as well).
    
    To avoid sending the problematic query names over DNS, commit
    6ca53a2453598804a2559a548a08424fca96434a ("resolv: Do not send queries
    for non-host-names in nss_dns [BZ #24112]") is needed.

Diff:
---
 ChangeLog                    | 33 ++++++++++++++++++++
 NEWS                         |  4 +++
 include/arpa/inet.h          |  6 ++--
 nscd/gai.c                   |  1 -
 nscd/gethstbynm3_r.c         |  2 --
 nss/digits_dots.c            |  3 +-
 resolv/Versions              |  1 +
 resolv/inet_addr.c           | 73 ++++++++++++++++++++++++++++----------------
 resolv/res_init.c            | 17 +++++++----
 resolv/tst-aton.c            | 35 +++++++++++++++++----
 resolv/tst-inet_aton_exact.c | 47 ++++++++++++++++++++++++++++
 sysdeps/posix/getaddrinfo.c  |  2 +-
 12 files changed, 177 insertions(+), 47 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c9ab984324..7cf21922f0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,36 @@
+2019-01-21  Florian Weimer  <fweimer@redhat.com>
+
+	[BZ #20018]
+	CVE-2016-10739
+	resolv: Reject trailing characters in host names
+	* include/arpa/inet.h (__inet_aton_exact): Declare.
+	(inet_aton): Remove hidden prototype.  No longer used internally.
+	* nscd/gai.c (__inet_aton): Do not define.
+	* nscd/gethstbynm3_r.c (__inet_aton): Likewise.
+	* nss/digits_dots.c (__inet_aton): Likewise.
+	(__nss_hostname_digits_dots_context): Call __inet_aton_exact.
+	* resolv/Makefile (tests-internal): Add tst-inet_aton_exact.
+	(tests): Add tst-resolv-nondecimal, tst-resolv-trailing.
+	(tst-resolv-nondecimal): Link with libresolv.so and libpthread.
+	(tst-resolv-trailing): Likewise.
+	* resolv/Versions (GLIBC_PRIVATE): Export __inet_aton_exact from
+	libc.
+	* resolv/inet_addr.c (inet_aton_end): Remame from __inet_aton.
+	Make static.  Add endp parameter.
+	(__inet_aton_exact): New function.
+	(__inet_aton_ignore_trailing): New function, aliased to inet_aton.
+	(__inet_addr): Call inet_aton_end.
+	* resolv/res_init.c (res_vinit_1): Truncate nameserver for IPv4,
+	not just IPv6.  Call __inet_aton_exact.
+	* resolv/tst-aton.c: Switch to <support/test-driver.c>.
+	(tests): Make const.  Add additional test cases with trailing
+	characters.
+	(do_test): Use array_length.
+	* resolv/tst-inet_aton_exact.c: New file.
+	* resolv/tst-resolv-trailing.c: Likewise.
+	* resolv/tst-resolv-nondecimal.c: Likewise.
+	* sysdeps/posix/getaddrinfo.c (gaih_inet): Call __inet_aton_exact.
+
 2018-05-18  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #22639]
diff --git a/NEWS b/NEWS
index 4be0b9891b..162a845cc0 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,10 @@ Deprecated and removed features, and other changes affecting compatibility:
 
 Security related changes:
 
+  CVE-2016-10739: The getaddrinfo function could successfully parse IPv4
+  addresses with arbitrary trailing characters, potentially leading to data
+  or command injection issues in applications.
+
   CVE-2017-18269: An SSE2-based memmove implementation for the i386
   architecture could corrupt memory.  Reported by Max Horn.
 
diff --git a/include/arpa/inet.h b/include/arpa/inet.h
index c3f28f2baa..19aec74275 100644
--- a/include/arpa/inet.h
+++ b/include/arpa/inet.h
@@ -1,10 +1,10 @@
 #include <inet/arpa/inet.h>
 
 #ifndef _ISOMAC
-extern int __inet_aton (const char *__cp, struct in_addr *__inp);
-libc_hidden_proto (__inet_aton)
+/* Variant of inet_aton which rejects trailing garbage.  */
+extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp);
+libc_hidden_proto (__inet_aton_exact)
 
-libc_hidden_proto (inet_aton)
 libc_hidden_proto (inet_ntop)
 libc_hidden_proto (inet_pton)
 extern __typeof (inet_pton) __inet_pton;
diff --git a/nscd/gai.c b/nscd/gai.c
index 576fd0045b..671169a927 100644
--- a/nscd/gai.c
+++ b/nscd/gai.c
@@ -19,7 +19,6 @@
 
 /* This file uses the getaddrinfo code but it compiles it without NSCD
    support.  We just need a few symbol renames.  */
-#define __inet_aton inet_aton
 #define __ioctl ioctl
 #define __getsockname getsockname
 #define __socket socket
diff --git a/nscd/gethstbynm3_r.c b/nscd/gethstbynm3_r.c
index 7beb9dce9f..f792c4fcd0 100644
--- a/nscd/gethstbynm3_r.c
+++ b/nscd/gethstbynm3_r.c
@@ -38,8 +38,6 @@
 #define HAVE_LOOKUP_BUFFER	1
 #define HAVE_AF			1
 
-#define __inet_aton inet_aton
-
 /* We are nscd, so we don't want to be talking to ourselves.  */
 #undef	USE_NSCD
 
diff --git a/nss/digits_dots.c b/nss/digits_dots.c
index 39bff38865..5441bce16e 100644
--- a/nss/digits_dots.c
+++ b/nss/digits_dots.c
@@ -29,7 +29,6 @@
 #include "nsswitch.h"
 
 #ifdef USE_NSCD
-# define inet_aton __inet_aton
 # include <nscd/nscd_proto.h>
 #endif
 
@@ -160,7 +159,7 @@ __nss_hostname_digits_dots_context (struct resolv_context *ctx,
 		     255.255.255.255?  The test below will succeed
 		     spuriously... ???  */
 		  if (af == AF_INET)
-		    ok = __inet_aton (name, (struct in_addr *) host_addr);
+		    ok = __inet_aton_exact (name, (struct in_addr *) host_addr);
 		  else
 		    {
 		      assert (af == AF_INET6);
diff --git a/resolv/Versions b/resolv/Versions
index b05778d965..9a82704af7 100644
--- a/resolv/Versions
+++ b/resolv/Versions
@@ -27,6 +27,7 @@ libc {
     __h_errno; __resp;
 
     __res_iclose;
+    __inet_aton_exact;
     __inet_pton_length;
     __resolv_context_get;
     __resolv_context_get_preinit;
diff --git a/resolv/inet_addr.c b/resolv/inet_addr.c
index 022f7ea084..d5f228d7de 100644
--- a/resolv/inet_addr.c
+++ b/resolv/inet_addr.c
@@ -78,29 +78,14 @@
 #include <limits.h>
 #include <errno.h>
 
-/*
- * Ascii internet address interpretation routine.
- * The value returned is in network order.
- */
-in_addr_t
-__inet_addr(const char *cp) {
-	struct in_addr val;
-
-	if (__inet_aton(cp, &val))
-		return (val.s_addr);
-	return (INADDR_NONE);
-}
-weak_alias (__inet_addr, inet_addr)
-
-/*
- * Check whether "cp" is a valid ascii representation
- * of an Internet address and convert to a binary address.
- * Returns 1 if the address is valid, 0 if not.
- * This replaces inet_addr, the return value from which
- * cannot distinguish between failure and a local broadcast address.
- */
-int
-__inet_aton(const char *cp, struct in_addr *addr)
+/* Check whether "cp" is a valid ASCII representation of an IPv4
+   Internet address and convert it to a binary address.  Returns 1 if
+   the address is valid, 0 if not.  This replaces inet_addr, the
+   return value from which cannot distinguish between failure and a
+   local broadcast address.  Write a pointer to the first
+   non-converted character to *endp.  */
+static int
+inet_aton_end (const char *cp, struct in_addr *addr, const char **endp)
 {
 	static const in_addr_t max[4] = { 0xffffffff, 0xffffff, 0xffff, 0xff };
 	in_addr_t val;
@@ -170,6 +155,7 @@ __inet_aton(const char *cp, struct in_addr *addr)
 
 	if (addr != NULL)
 		addr->s_addr = res.word | htonl (val);
+	*endp = cp;
 
 	__set_errno (saved_errno);
 	return (1);
@@ -178,6 +164,41 @@ ret_0:
 	__set_errno (saved_errno);
 	return (0);
 }
-weak_alias (__inet_aton, inet_aton)
-libc_hidden_def (__inet_aton)
-libc_hidden_weak (inet_aton)
+
+int
+__inet_aton_exact (const char *cp, struct in_addr *addr)
+{
+  struct in_addr val;
+  const char *endp;
+  /* Check that inet_aton_end parsed the entire string.  */
+  if (inet_aton_end (cp, &val, &endp) != 0 && *endp == 0)
+    {
+      *addr = val;
+      return 1;
+    }
+  else
+    return 0;
+}
+libc_hidden_def (__inet_aton_exact)
+
+/* inet_aton ignores trailing garbage.  */
+int
+__inet_aton_ignore_trailing (const char *cp, struct in_addr *addr)
+{
+  const char *endp;
+  return  inet_aton_end (cp, addr, &endp);
+}
+weak_alias (__inet_aton_ignore_trailing, inet_aton)
+
+/* ASCII IPv4 Internet address interpretation routine.  The value
+   returned is in network order.  */
+in_addr_t
+__inet_addr (const char *cp)
+{
+  struct in_addr val;
+  const char *endp;
+  if (inet_aton_end (cp, &val, &endp))
+    return val.s_addr;
+  return INADDR_NONE;
+}
+weak_alias (__inet_addr, inet_addr)
diff --git a/resolv/res_init.c b/resolv/res_init.c
index f5e52cbbb9..94743a252e 100644
--- a/resolv/res_init.c
+++ b/resolv/res_init.c
@@ -399,8 +399,16 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
               cp = parser->buffer + sizeof ("nameserver") - 1;
               while (*cp == ' ' || *cp == '\t')
                 cp++;
+
+              /* Ignore trailing contents on the name server line.  */
+              {
+                char *el;
+                if ((el = strpbrk (cp, " \t\n")) != NULL)
+                  *el = '\0';
+              }
+
               struct sockaddr *sa;
-              if ((*cp != '\0') && (*cp != '\n') && __inet_aton (cp, &a))
+              if ((*cp != '\0') && (*cp != '\n') && __inet_aton_exact (cp, &a))
                 {
                   sa = allocate_address_v4 (a, NAMESERVER_PORT);
                   if (sa == NULL)
@@ -410,9 +418,6 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
                 {
                   struct in6_addr a6;
                   char *el;
-
-                  if ((el = strpbrk (cp, " \t\n")) != NULL)
-                    *el = '\0';
                   if ((el = strchr (cp, SCOPE_DELIMITER)) != NULL)
                     *el = '\0';
                   if ((*cp != '\0') && (__inet_pton (AF_INET6, cp, &a6) > 0))
@@ -472,7 +477,7 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
                   char separator = *cp;
                   *cp = 0;
                   struct resolv_sortlist_entry e;
-                  if (__inet_aton (net, &a))
+                  if (__inet_aton_exact (net, &a))
                     {
                       e.addr = a;
                       if (is_sort_mask (separator))
@@ -484,7 +489,7 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
                             cp++;
                           separator = *cp;
                           *cp = 0;
-                          if (__inet_aton (net, &a))
+                          if (__inet_aton_exact (net, &a))
                             e.mask = a.s_addr;
                           else
                             e.mask = net_mask (e.addr);
diff --git a/resolv/tst-aton.c b/resolv/tst-aton.c
index 08110a007a..eb734d7758 100644
--- a/resolv/tst-aton.c
+++ b/resolv/tst-aton.c
@@ -1,11 +1,29 @@
+/* Test legacy IPv4 text-to-address function inet_aton.
+   Copyright (C) 1998-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <array_length.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
-
-static struct tests
+static const struct tests
 {
   const char *input;
   int valid;
@@ -16,6 +34,7 @@ static struct tests
   { "-1", 0, 0 },
   { "256", 1, 0x00000100 },
   { "256.", 0, 0 },
+  { "255a", 0, 0 },
   { "256a", 0, 0 },
   { "0x100", 1, 0x00000100 },
   { "0200.0x123456", 1, 0x80123456 },
@@ -40,7 +59,12 @@ static struct tests
   { "1.2.256.4", 0, 0 },
   { "1.2.3.0x100", 0, 0 },
   { "323543357756889", 0, 0 },
-  { "10.1.2.3.4", 0, 0},
+  { "10.1.2.3.4", 0, 0 },
+  { "192.0.2.1", 1, 0xc0000201 },
+  { "192.0.2.2\nX", 1, 0xc0000202 },
+  { "192.0.2.3 Y", 1, 0xc0000203 },
+  { "192.0.2.3Z", 0, 0 },
+  { "192.000.002.010", 1, 0xc0000208 },
 };
 
 
@@ -50,7 +74,7 @@ do_test (void)
   int result = 0;
   size_t cnt;
 
-  for (cnt = 0; cnt < sizeof (tests) / sizeof (tests[0]); ++cnt)
+  for (cnt = 0; cnt < array_length (tests); ++cnt)
     {
       struct in_addr addr;
 
@@ -73,5 +97,4 @@ do_test (void)
   return result;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/resolv/tst-inet_aton_exact.c b/resolv/tst-inet_aton_exact.c
new file mode 100644
index 0000000000..0fdfa3d6aa
--- /dev/null
+++ b/resolv/tst-inet_aton_exact.c
@@ -0,0 +1,47 @@
+/* Test internal legacy IPv4 text-to-address function __inet_aton_exact.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <arpa/inet.h>
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+  struct in_addr addr = { };
+
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.1", &addr), 1);
+  TEST_COMPARE (ntohl (addr.s_addr), 0xC0000201);
+
+  TEST_COMPARE (__inet_aton_exact ("192.000.002.010", &addr), 1);
+  TEST_COMPARE (ntohl (addr.s_addr), 0xC0000208);
+  TEST_COMPARE (__inet_aton_exact ("0xC0000234", &addr), 1);
+  TEST_COMPARE (ntohl (addr.s_addr), 0xC0000234);
+
+  /* Trailing content is not accepted.  */
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.2X", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.3 Y", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.4\nZ", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.5\tT", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.6 Y", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.7\n", &addr), 0);
+  TEST_COMPARE (__inet_aton_exact ("192.0.2.8\t", &addr), 0);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index c15f76e547..d0429b2206 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -504,7 +504,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
 	}
 #endif
 
-      if (__inet_aton (name, (struct in_addr *) at->addr) != 0)
+      if (__inet_aton_exact (name, (struct in_addr *) at->addr) != 0)
 	{
 	  if (req->ai_family == AF_UNSPEC || req->ai_family == AF_INET)
 	    at->family = AF_INET;