[PATCH] nss_dns: Check address length before creating addrinfo result [BZ #19831]

[PATCH] nss_dns: Check address length before creating addrinfo result [BZ #19831]

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 192 bytes --]

This also consolidates the behavior between single (A or AAAA) and
dual (A and AAAA in parallel) queries.  Single queries checked
the record length against the QTYPE, not the RRTYPE.

Florian

[-- Attachment #2: 0001-nss_dns-Check-address-length-before-creating-addrinf.patch --]
[-- Type: text/x-patch, Size: 3227 bytes --]

2016-03-25  Florian Weimer  <fweimer@redhat.com>

	[BZ #19831]
	* resolv/nss_dns/dns-host.c (rrtype_to_rdata_length): New
	function.
	(getanswer_r): Check RDATA length against RRTYPE and QTYPE.
	(gaih_getanswer_slice): Check RDATA length against RRTYPE.

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index 4bb0e62..ebe5372 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -139,6 +139,22 @@ extern enum nss_status _nss_dns_gethostbyname3_r (const char *name, int af,
 						  char **canonp);
 hidden_proto (_nss_dns_gethostbyname3_r)
 
+/* Return the expected RDATA length for an address record type (A or
+   AAAA).  */
+static int
+rrtype_to_rdata_length (int type)
+{
+  switch (type)
+    {
+    case T_A:
+      return INADDRSZ;
+    case T_AAAA:
+      return IN6ADDRSZ;
+    default:
+      return -1;
+    }
+}
+
 enum nss_status
 _nss_dns_gethostbyname3_r (const char *name, int af, struct hostent *result,
 			   char *buffer, size_t buflen, int *errnop,
@@ -903,6 +919,15 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype,
 	      cp += n;
 	      continue;			/* XXX - had_error++ ? */
 	    }
+
+	  /* Stop parsing at a record whose length is incorrect.  */
+	  if (n != rrtype_to_rdata_length (type))
+	    {
+	      ++had_error;
+	      break;
+	    }
+
+	  /* Skip records of the wrong type.  */
 	  if (n != result->h_length)
 	    {
 	      cp += n;
@@ -1139,32 +1164,26 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname,
 	    }
 	  continue;
 	}
-#if 1
-      // We should not see any types other than those explicitly listed
-      // below.  Some types sent by server seem missing, though.  Just
-      // collect the data for now.
-      if (__glibc_unlikely (type != T_A && type != T_AAAA))
-#else
-      if (__builtin_expect (type == T_SIG, 0)
-	  || __builtin_expect (type == T_KEY, 0)
-	  || __builtin_expect (type == T_NXT, 0)
-	  || __builtin_expect (type == T_PTR, 0)
-	  || __builtin_expect (type == T_DNAME, 0))
-#endif
-	{
-	  /* We don't support DNSSEC yet.  For now, ignore the record
-	     and send a low priority message to syslog.
 
-	     We also don't expect T_PTR or T_DNAME messages.  */
-	  syslog (LOG_DEBUG | LOG_AUTH,
-		  "getaddrinfo*.gaih_getanswer: got type \"%s\"",
-		  p_type (type));
+
+      /* Stop parsing if we encounter a record with incorrect RDATA
+	 length.  */
+      if (type == T_A || type == T_AAAA)
+	{
+	  if (n != rrtype_to_rdata_length (type))
+	    {
+	      ++had_error;
+	      continue;
+	    }
+	}
+      else
+	{
+	  /* Skip unknown records.  */
 	  cp += n;
 	  continue;
 	}
-      if (type != T_A && type != T_AAAA)
-	abort ();
 
+      assert (type == T_A || type == T_AAAA);
       if (*pat == NULL)
 	{
 	  uintptr_t pad = (-(uintptr_t) buffer
@@ -1198,12 +1217,6 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname,
 	}
 
       (*pat)->family = type == T_A ? AF_INET : AF_INET6;
-      if (__builtin_expect ((type == T_A && n != INADDRSZ)
-			    || (type == T_AAAA && n != IN6ADDRSZ), 0))
-	{
-	  ++had_error;
-	  continue;
-	}
       memcpy ((*pat)->addr, cp, n);
       cp += n;
       (*pat)->scopeid = 0;

Re: [PATCH] nss_dns: Check address length before creating addrinfo result [BZ #19831]

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 281 bytes --]

On 03/25/2016 07:59 PM, Florian Weimer wrote:
> This also consolidates the behavior between single (A or AAAA) and
> dual (A and AAAA in parallel) queries.  Single queries checked
> the record length against the QTYPE, not the RRTYPE.

Here is what I checked in.

Thanks,
Florian


[-- Attachment #2: 0001-nss_dns-Check-address-length-before-creating-addrinf.patch --]
[-- Type: text/x-patch, Size: 2940 bytes --]

2016-04-27  Florian Weimer  <fweimer@redhat.com>

	[BZ #19831]
	* resolv/nss_dns/dns-host.c (rrtype_to_rdata_length): New
	function.
	(getanswer_r): Check RDATA length against RRTYPE and QTYPE.
	(gaih_getanswer_slice): Check RDATA length against RRTYPE.

diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index fb1d21c..403a005 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -134,6 +134,22 @@ extern enum nss_status _nss_dns_gethostbyname3_r (const char *name, int af,
 						  char **canonp);
 hidden_proto (_nss_dns_gethostbyname3_r)
 
+/* Return the expected RDATA length for an address record type (A or
+   AAAA).  */
+static int
+rrtype_to_rdata_length (int type)
+{
+  switch (type)
+    {
+    case T_A:
+      return INADDRSZ;
+    case T_AAAA:
+      return IN6ADDRSZ;
+    default:
+      return -1;
+    }
+}
+
 enum nss_status
 _nss_dns_gethostbyname3_r (const char *name, int af, struct hostent *result,
 			   char *buffer, size_t buflen, int *errnop,
@@ -888,6 +904,15 @@ getanswer_r (const querybuf *answer, int anslen, const char *qname, int qtype,
 	      cp += n;
 	      continue;			/* XXX - had_error++ ? */
 	    }
+
+	  /* Stop parsing at a record whose length is incorrect.  */
+	  if (n != rrtype_to_rdata_length (type))
+	    {
+	      ++had_error;
+	      break;
+	    }
+
+	  /* Skip records of the wrong type.  */
 	  if (n != result->h_length)
 	    {
 	      cp += n;
@@ -1124,25 +1149,25 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname,
 	    }
 	  continue;
 	}
-#if 1
-      // We should not see any types other than those explicitly listed
-      // below.  Some types sent by server seem missing, though.  Just
-      // collect the data for now.
-      if (__glibc_unlikely (type != T_A && type != T_AAAA))
-#else
-      if (__builtin_expect (type == T_SIG, 0)
-	  || __builtin_expect (type == T_KEY, 0)
-	  || __builtin_expect (type == T_NXT, 0)
-	  || __builtin_expect (type == T_PTR, 0)
-	  || __builtin_expect (type == T_DNAME, 0))
-#endif
+
+      /* Stop parsing if we encounter a record with incorrect RDATA
+	 length.  */
+      if (type == T_A || type == T_AAAA)
+	{
+	  if (n != rrtype_to_rdata_length (type))
+	    {
+	      ++had_error;
+	      continue;
+	    }
+	}
+      else
 	{
+	  /* Skip unknown records.  */
 	  cp += n;
 	  continue;
 	}
-      if (type != T_A && type != T_AAAA)
-	abort ();
 
+      assert (type == T_A || type == T_AAAA);
       if (*pat == NULL)
 	{
 	  uintptr_t pad = (-(uintptr_t) buffer
@@ -1176,12 +1201,6 @@ gaih_getanswer_slice (const querybuf *answer, int anslen, const char *qname,
 	}
 
       (*pat)->family = type == T_A ? AF_INET : AF_INET6;
-      if (__builtin_expect ((type == T_A && n != INADDRSZ)
-			    || (type == T_AAAA && n != IN6ADDRSZ), 0))
-	{
-	  ++had_error;
-	  continue;
-	}
       memcpy ((*pat)->addr, cp, n);
       cp += n;
       (*pat)->scopeid = 0;