[PATCH] Fix initshells

[PATCH] Fix initshells

[Jakub Jelinek]

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

Hi!

If /etc/shells ends with an empty line, initshells ends with an endless
loop (because fgets with length 1 always succeeds, doesn't need to read
anything from the file, just stores '\0').  In addition to this
if /etc/shells contains e.g.
/
/
/
/
/
/
/
/
then setusershell (); endusershell (); causes heap corruption.
I'm attaching two patches, some fixes are common to both patches, but
they differ in how the endless loop with empty newline at end is solved.
P3 adds an extra condition to the loop, P5 increases the strings
buffer by one byte, so that fgets itself cures this.
I have also coded up a third solution, as we never shrink the strings
buffer, we might as well just fread the whole file into the strings
buffer and instead of fgets simply look for '\n's in it.  While that
is tiny bit more efficient, it compiled into bigger code (on x86-64)
and I believe for this type of functions smaller code is better code.

	Jakub

[-- Attachment #2: P3 --]
[-- Type: text/plain, Size: 1544 bytes --]

2006-12-09  Jakub Jelinek  <jakub@redhat.com>

	* misc/getusershell.c (initshells): Check for integer overflows.
	Avoid endless loop if /etc/shells ends with an empty line.
	Don't use calloc for shells array.  Disallow / as shell.

--- libc/misc/getusershell.c.jj	2006-05-15 20:56:36.000000000 +0200
+++ libc/misc/getusershell.c	2006-12-09 12:16:30.000000000 +0100
@@ -98,7 +98,7 @@ initshells()
 	register char **sp, *cp;
 	register FILE *fp;
 	struct stat64 statb;
-	int flen;
+	size_t flen;
 
 	free(shells);
 	shells = NULL;
@@ -114,9 +114,11 @@ initshells()
 		okshells[1] = _PATH_CSHELL;
 		return (char **) okshells;
 	}
-	if ((strings = malloc((u_int)statb.st_size + 1)) == NULL)
+	if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
 		goto init_okshells;
-	shells = calloc((unsigned)statb.st_size / 3, sizeof (char *));
+	if ((strings = malloc(statb.st_size + 1)) == NULL)
+		goto init_okshells;
+	shells = malloc(statb.st_size / 3 * sizeof (char *));
 	if (shells == NULL) {
 		free(strings);
 		strings = NULL;
@@ -124,11 +126,12 @@ initshells()
 	}
 	sp = shells;
 	cp = strings;
-	flen = statb.st_size;
-	while (fgets_unlocked(cp, flen - (cp - strings), fp) != NULL) {
+	flen = statb.st_size + 1;
+	while (cp < strings + flen - 1
+	       && fgets_unlocked(cp, flen - (cp - strings), fp) != NULL) {
 		while (*cp != '#' && *cp != '/' && *cp != '\0')
 			cp++;
-		if (*cp == '#' || *cp == '\0')
+		if (*cp == '#' || *cp == '\0' || cp[1] == '\0')
 			continue;
 		*sp++ = cp;
 		while (!isspace(*cp) && *cp != '#' && *cp != '\0')

[-- Attachment #3: P5 --]
[-- Type: text/plain, Size: 1468 bytes --]

2006-12-09  Jakub Jelinek  <jakub@redhat.com>

	* misc/getusershell.c (initshells): Check for integer overflows.
	Make strings buffer one bigger as fgets always succeeds when second
	argument is 1.  Don't use calloc for shells array.  Disallow
	/ as shell.

--- libc/misc/getusershell.c.jj	2006-05-15 20:56:36.000000000 +0200
+++ libc/misc/getusershell.c	2006-12-09 12:53:05.000000000 +0100
@@ -98,7 +98,7 @@ initshells()
 	register char **sp, *cp;
 	register FILE *fp;
 	struct stat64 statb;
-	int flen;
+	size_t flen;
 
 	free(shells);
 	shells = NULL;
@@ -114,9 +114,11 @@ initshells()
 		okshells[1] = _PATH_CSHELL;
 		return (char **) okshells;
 	}
-	if ((strings = malloc((u_int)statb.st_size + 1)) == NULL)
+	if (statb.st_size > ~(size_t)0 / sizeof (char *) * 3)
 		goto init_okshells;
-	shells = calloc((unsigned)statb.st_size / 3, sizeof (char *));
+	if ((strings = malloc(statb.st_size + 2)) == NULL)
+		goto init_okshells;
+	shells = malloc(statb.st_size / 3 * sizeof (char *));
 	if (shells == NULL) {
 		free(strings);
 		strings = NULL;
@@ -124,11 +126,11 @@ initshells()
 	}
 	sp = shells;
 	cp = strings;
-	flen = statb.st_size;
+	flen = statb.st_size + 2;
 	while (fgets_unlocked(cp, flen - (cp - strings), fp) != NULL) {
 		while (*cp != '#' && *cp != '/' && *cp != '\0')
 			cp++;
-		if (*cp == '#' || *cp == '\0')
+		if (*cp == '#' || *cp == '\0' || cp[1] == '\0')
 			continue;
 		*sp++ = cp;
 		while (!isspace(*cp) && *cp != '#' && *cp != '\0')