Fix BZ #18660 -- overflow in getusershell

  Greetings,

Attached patch fixes BZ #18660 -- overflow in getusershell.

Note: the file does not follow GNU coding convention, should I make a
whitespace-only pass over it?
Note: Tobias proposed a different patch:
https://sourceware.org/bugzilla/attachment.cgi?id=8502, but I like my
patch better.

P.S. AFAICT, this is nearly impossible to test :-(

Thanks,

2015-08-15  Paul Pluzhnikov  <ppluzhnikov@google.com>
            Tobias Stoeckmann <tobias@stoeckmann.org>

        [BZ #18660]
        * misc/getusershell.c (initshells): Fix possible overflow.
  

Since you're increasing an allocation size, don't you also need to adjust 
the check a few lines earlier for whether the allocation size calculation 
would overflow?
  

> On August 17, 2015 at 12:59 PM Joseph Myers <joseph@codesourcery.com> wrote:
> Since you're increasing an allocation size, don't you also need to adjust 
> the check a few lines earlier for whether the allocation size calculation 
> would overflow?

	if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
		goto init_okshells;
	flen = statb.st_size + 3;

The check has to focus on flen's statb.st_size + 3 anyway.
It's larger than statb.st_size / 3.
  

On Mon, 17 Aug 2015, Tobias Stöckmann wrote:

> > On August 17, 2015 at 12:59 PM Joseph Myers <joseph@codesourcery.com> wrote:
> > Since you're increasing an allocation size, don't you also need to adjust 
> > the check a few lines earlier for whether the allocation size calculation 
> > would overflow?
> 
> 	if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
> 		goto init_okshells;
> 	flen = statb.st_size + 3;
> 
> The check has to focus on flen's statb.st_size + 3 anyway.
> It's larger than statb.st_size / 3.

It's smaller than statb.st_size / 3 * sizeof (char *) (or the new adjusted 
version thereof), since sizeof (char *) is 4 or 8 for configurations 
supported by glibc.
  

> On August 17, 2015 at 5:48 PM Joseph Myers <joseph@codesourcery.com> wrote:
> It's smaller than statb.st_size / 3 * sizeof (char *) (or the new adjusted 
> version thereof), since sizeof (char *) is 4 or 8 for configurations 
> supported by glibc.

I wonder what makes me blind towards "sizeof (char *)" these days.
You are right, of course.
  

On 15 Aug 2015 11:31, Paul Pluzhnikov wrote:
> P.S. AFAICT, this is nearly impossible to test :-(

it is if we get clever/abusive.  if you change misc/getusershell.c like so:
+#ifndef _LOCAL_PATH_SHELLS
+# define _LOCAL_PATH_SHELLS _PATH_SHELLS
+#endif
-	if ((fp = fopen(_PATH_SHELLS, "rce")) == NULL)
+	if ((fp = fopen(_LOCAL_PATH_SHELLS, "rce")) == NULL)

then add -D_LOCAL_PATH_SHELLS='"$(srcdir)/tst-shells.conf"' to the test's
CPPFLAGS and list misc/getusershell.c directly in the test lines.  that
way your test code will use those funcs directly with your custom shell
path.
-mike