getpass fgets check

[Peter Mazinger]

Hi,

this way we do not use the PWD_BUFFER_SIZE dependency, either we ignore it (and remove it as config option) or we consider it mandatory if defined

Regards, Peter
-------- Original-Nachricht --------
> Datum: Tue, 20 Dec 2011 06:40:28 -0600
> Von: William Pitcock <nenolod at dereferenced.org>
> An: Joakim Tjernlund <joakim.tjernlund at transmode.se>
> CC: uclibc at uclibc.org
> Betreff: Re: getpass fgets check

> Hi,
> 
> On Tue, Dec 20, 2011 at 2:43 AM, Joakim Tjernlund
> <joakim.tjernlund at transmode.se> wrote:
> >> From: Daniel Wainwright <wainwright.daniel at gmail.com>
> >> To: uclibc at uclibc.org
> >> Date: 2011/12/20 08:44
> >> Subject: getpass fgets check
> >> Sent by: uclibc-bounces at uclibc.org
> >>
> >> Hi,
> >>
> >> I believe there is a simple error in getpass.c, line 80:
> >>
> >>
> >>
> >>   static char buf[PWD_BUFFER_SIZE];
> >>
> >>   ...
> >>
> >>   /* Read the password.  */
> >>   fgets (buf, PWD_BUFFER_SIZE-1, in);
> >>   if (buf != NULL)
> >>
> >>   ...
> >>
> >>
> >>
> >> So the result of fgets is not being checked here, results in reading
> the
> >> buffer uninitialised below.
> >
> > yes, and I think(if max passwd len is important) that it should read
> >  fgets (buf, PWD_BUFFER_SIZE, in)
> > as fgets man page says:
> >       fgets() reads in at most one less than size characters from
> stream  and
> >       stores  them  into  the buffer pointed to by s.
> 
> I think that using 'sizeof buf' is cleaner and more futureproof than
> depending on PWD_BUFFER_SIZE.  Something like this should cleanly fix
> the potential usage of buf whilst in uninitialized state:
> 
> diff --git a/libc/unistd/getpass.c b/libc/unistd/getpass.c
> index 8d80182..b8cb640 100644
> --- a/libc/unistd/getpass.c
> +++ b/libc/unistd/getpass.c
> @@ -76,9 +76,11 @@ char * getpass (const char *prompt)
>    fputs(prompt, out);
>    fflush(out);
> 
> -  /* Read the password.  */
> -  fgets (buf, PWD_BUFFER_SIZE-1, in);
> -  if (buf != NULL)
> +  /* Read the password, ensuring buf is initialized first as fgets() may
> not
> +     touch the buffer itself. */
> +  *buf = '\0';
> +  fgets (buf, sizeof buf, in);
> +  if (*buf != '\0')
>      {
>        nread = strlen(buf);
>        if (nread < 0)
> 
> Please test and see if that fixes your problem.
> 
> William
> _______________________________________________
> uClibc mailing list
> uClibc at uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc

-- 
NEU: FreePhone - 0ct/min Handyspartarif mit Geld-zurück-Garantie!		
Jetzt informieren: http://www.gmx.net/de/go/freephone