[PATCH] nss_dns: Adjust ns_name_ntop failure handling in getnetby*

[PATCH] nss_dns: Adjust ns_name_ntop failure handling in getnetby*

[Florian Weimer]

If the function fails, the output buffer is not large enough.  The
function does not fail for any other reason.

2019-03-08  Florian Weimer  <fweimer@redhat.com>

	* resolv/nss_dns/dns-network.c (getanswer_r): On ns_name_ntop
	failure, the buffer is always too small.

diff --git a/resolv/nss_dns/dns-network.c b/resolv/nss_dns/dns-network.c
index 68266d57c9..9b75ac8233 100644
--- a/resolv/nss_dns/dns-network.c
+++ b/resolv/nss_dns/dns-network.c
@@ -324,19 +324,14 @@ getanswer_r (const querybuf *answer, int anslen, struct netent *result,
     {
       int n = __ns_name_unpack (answer->buf, end_of_message, cp,
 				packtmp, sizeof packtmp);
-      if (n != -1 && __ns_name_ntop (packtmp, bp, linebuflen) == -1)
-	{
-	  if (errno == EMSGSIZE)
-	    goto too_small;
-
-	  n = -1;
-	}
-
-      if (n > 0 && bp[0] == '.')
-	bp[0] = '\0';
-
       if (n < 0)
 	goto bad_message;
+      if (__ns_name_ntop (packtmp, bp, linebuflen) < 0)
+	goto too_small;
+
+      if (bp[0] == '.')
+	bp[0] = '\0';
+
       cp += n;
 
       if (end_of_message - cp < 10)
@@ -355,16 +350,11 @@ getanswer_r (const querybuf *answer, int anslen, struct netent *result,
 	{
 	  n = __ns_name_unpack (answer->buf, end_of_message, cp,
 				packtmp, sizeof packtmp);
-	  if (n != -1 && __ns_name_ntop (packtmp, bp, linebuflen) == -1)
-	    {
-	      if (errno == EMSGSIZE)
-		goto too_small;
-
-	      n = -1;
-	    }
-
 	  if (n < 0)
 	    goto bad_message;
+	  if (__ns_name_ntop (packtmp, bp, linebuflen) < 0)
+	    goto too_small;
+
 	  cp += rdatalen;
          if (alias_pointer + 2 < &net_data->aliases[MAX_NR_ALIASES])
            {

Re: [PATCH] nss_dns: Adjust ns_name_ntop failure handling in getnetby*

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> -      if (n > 0 && bp[0] == '.')
> -	bp[0] = '\0';
> -

> +      if (bp[0] == '.')
> +	bp[0] = '\0';

This now references bp[0] when n == 0, which it didn't before.  But,
ns_name_ntop always nul terminates the buffer, so bp[0] is always
well-defined.  OK.  What about the case where __ns_name_unpack() returns
zero?  It looks like that might have been a bug in the old version?

Otherwise LGTM.

Re: [PATCH] nss_dns: Adjust ns_name_ntop failure handling in getnetby*

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>> -      if (n > 0 && bp[0] == '.')
>> -	bp[0] = '\0';
>> -
>
>> +      if (bp[0] == '.')
>> +	bp[0] = '\0';
>
> This now references bp[0] when n == 0, which it didn't before.  But,
> ns_name_ntop always nul terminates the buffer, so bp[0] is always
> well-defined.  OK.  What about the case where __ns_name_unpack() returns
> zero?  It looks like that might have been a bug in the old version?

ns_name_unpack returns -1 on failure, and on success the length of the
name or name fragment in the original message.  That can never be zero.
It always includes at least the null terminate (one byte) or the
compression reference (two bytes).

The other question is whether it makes sense to rewrite "." (the only
possible output with a leading dot) to "".  I have really no idea why
the code does this.  It may be an attempt to remove a trailing dot '.',
but ns_name_ntop does not produce such names.

Hmm, I wonder if the "" case is one where the res_dnok actually fails.

Thanks,
Florian