nisplus: Correct pwent parsing issue and resulting compilation error

  Copy and null-terminate NIS+ password file UID and GID entries whose 
length is non-zero rather than zero, fixing a bug and a compilation 
issue causing an error with GCC 8:

nss_nisplus/nisplus-parser.c: In function '_nss_nisplus_parse_pwent':
nss_nisplus/nisplus-parser.c:90:7: error: 'strncpy' destination unchanged after copying no bytes [-Werror=stringop-truncation]
       strncpy (first_unused, numstr, len);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
nss_nisplus/nisplus-parser.c:106:7: error: 'strncpy' destination unchanged after copying no bytes [-Werror=stringop-truncation]
       strncpy (first_unused, numstr, len);
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

introduced with commit ac05397075f6:

commit ac05397075f621cfdbe1db527c96167a58b6d18e
Author: Ulrich Drepper <drepper@redhat.com>
Date:   Sun Apr 30 07:01:26 2006 +0000

	* nis/nss_nisplus/nisplus-parser.c: Minor optimizations and
	cleanups.  Avoid copying data if it can be used in the old place.

(no mailing list reference available).  Obviously regardless of the 
recently added compiler diagnostics causing a build error this code has 
been long non-functional, so I guess NIS+ servers have been supplying 
strings that have already been null-terminated.  Which in turn made it 
unnecessary to make a null-terminated copy, masking this bug.

	* nis/nss_nisplus/nisplus-parser.c (_nss_nisplus_parse_pwent):
	Copy and null-terminate entries whose length is non-zero rather 
	than zero.
---
Hi,

 No issues seen in `mips-linux-gnu' o32 testing.  OK to apply?

 Also this may have to be backported and security implications evaluated.

  Maciej
---
 nis/nss_nisplus/nisplus-parser.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

glibc-nisplus-parse-pwent-len.diff
  

On Mon, 18 Jun 2018, Maciej W. Rozycki wrote:

> Copy and null-terminate NIS+ password file UID and GID entries whose 
> length is non-zero rather than zero, fixing a bug and a compilation 
> issue causing an error with GCC 8:

Note: (a) the warnings are bug 23266, (b) we now have alternative 
candidate patches, as there's 
<https://sourceware.org/ml/libc-alpha/2018-04/msg00209.html> as well, (c) 
as I said in <https://sourceware.org/ml/libc-alpha/2018-03/msg00532.html> 
it would be a very good idea to add a --enable-obsolete-rpc 
--enable-obsolete-nsl configuration to build-many-glibcs.py so issues 
building such a configuration with new GCC get detected and fixed quicker.
  

On Jun 18 2018, "Maciej W. Rozycki" <macro@mips.com> wrote:

> Index: glibc/nis/nss_nisplus/nisplus-parser.c
> ===================================================================
> --- glibc.orig/nis/nss_nisplus/nisplus-parser.c	2018-02-14 14:57:00.000000000 +0000
> +++ glibc/nis/nss_nisplus/nisplus-parser.c	2018-06-16 21:47:29.071633078 +0100
> @@ -82,7 +82,7 @@ _nss_nisplus_parse_pwent (nis_result *re
>  
>    char *numstr = NISOBJVAL (2, obj);
>    len = NISOBJLEN (2, obj);
> -  if (len == 0 && numstr[len - 1] != '\0')
> +  if (len != 0 && numstr[len - 1] != '\0')
>      {
>        if (len >= room_left)
>  	goto no_more_room;
> @@ -98,7 +98,7 @@ _nss_nisplus_parse_pwent (nis_result *re
>  
>    numstr = NISOBJVAL (3, obj);
>    len = NISOBJLEN (3, obj);
> -  if (len == 0 && numstr[len - 1] != '\0')
> +  if (len != 0 && numstr[len - 1] != '\0')
>      {
>        if (len >= room_left)
>  	goto no_more_room;
>

I don't think this is correct.  If len == 0 then numstr[0] is undefined.

Andreas.
  

On Mon, 18 Jun 2018, Joseph Myers wrote:

> > Copy and null-terminate NIS+ password file UID and GID entries whose 
> > length is non-zero rather than zero, fixing a bug and a compilation 
> > issue causing an error with GCC 8:
> 
> Note: (a) the warnings are bug 23266, (b) we now have alternative 
> candidate patches, as there's 
> <https://sourceware.org/ml/libc-alpha/2018-04/msg00209.html> as well, (c) 

 Umm, I didn't notice `_nss_nisplus_parse_grent' suffers from the same 
problem and neither GCC bothered here for some reason.

> as I said in <https://sourceware.org/ml/libc-alpha/2018-03/msg00532.html> 
> it would be a very good idea to add a --enable-obsolete-rpc 
> --enable-obsolete-nsl configuration to build-many-glibcs.py so issues 
> building such a configuration with new GCC get detected and fixed quicker.

 Hmm, but why it's been 2+ months and such an obvious fix hasn't been 
reviewed yet?  Insufficient pinging?

  Maciej
  

On Mon, 18 Jun 2018, Maciej W. Rozycki wrote:

> > as I said in <https://sourceware.org/ml/libc-alpha/2018-03/msg00532.html> 
> > it would be a very good idea to add a --enable-obsolete-rpc 
> > --enable-obsolete-nsl configuration to build-many-glibcs.py so issues 
> > building such a configuration with new GCC get detected and fixed quicker.
> 
>  Hmm, but why it's been 2+ months and such an obvious fix hasn't been 
> reviewed yet?  Insufficient pinging?

Only pinged once as far as I can see.  Quite likely most developers aren't 
using those configure options and so aren't affected by the issue.