Tobias Stoeckmann wrote:
> There's still a possible overflow in getusershell.c. We could increase
> the buffer allocation yet again, but I have to agree with the glibc
> developers here: enough is enough. The code is ugly and has proven to be
> difficult to review.
Another approach is to rewrite the function to allocate memory as needed and
not play these games. This diff preserves the existing behavior, but doesn't
play 1985 era games trying to save 24 bytes of memory by compacting all the
strings into a single allocation.
Actually, I think this even uses *less* memory since the majority of my
/etc/shells is just comment lines that won't be preserved.
Index: gen/getusershell.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/getusershell.c,v
retrieving revision 1.16
diff -u -p -r1.16 getusershell.c
--- gen/getusershell.c 14 Sep 2015 16:09:13 -0000 1.16
+++ gen/getusershell.c 5 Dec 2015 16:08:56 -0000
@@ -28,14 +28,13 @@
* SUCH DAMAGE.
*/
-#include <sys/stat.h>
-
#include <ctype.h>
#include <limits.h>
#include <paths.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
#include <unistd.h>
/*
@@ -44,7 +43,7 @@
*/
static char *okshells[] = { _PATH_BSHELL, _PATH_CSHELL, _PATH_KSHELL, NULL };
-static char **curshell, **shells, *strings;
+static char **curshell, **shells;
static char **initshells(void);
/*
@@ -66,11 +65,14 @@ getusershell(void)
void
endusershell(void)
{
-
+ char **s;
+
+ if ((s = shells))
+ while (*s)
+ free(*s++);
free(shells);
shells = NULL;
- free(strings);
- strings = NULL;
+
curshell = NULL;
}
@@ -84,48 +86,52 @@ setusershell(void)
static char **
initshells(void)
{
- char **sp, *cp;
+ char buf[PATH_MAX];
+ int nshells = 0, nalloc;
+ char *cp;
FILE *fp;
- struct stat statb;
free(shells);
shells = NULL;
- free(strings);
- strings = NULL;
+
if ((fp = fopen(_PATH_SHELLS, "re")) == NULL)
return (okshells);
- if (fstat(fileno(fp), &statb) == -1) {
- (void)fclose(fp);
- return (okshells);
- }
- if (statb.st_size > SIZE_MAX) {
- (void)fclose(fp);
- return (okshells);
- }
- if ((strings = malloc((size_t)statb.st_size)) == NULL) {
- (void)fclose(fp);
- return (okshells);
- }
- shells = calloc((size_t)(statb.st_size / 3 + 2), sizeof (char *));
- if (shells == NULL) {
- (void)fclose(fp);
- free(strings);
- strings = NULL;
- return (okshells);
- }
- sp = shells;
- cp = strings;
- while (fgets(cp, PATH_MAX + 1, fp) != NULL) {
+
+ nalloc = 10; // just an initial guess
+ nshells = 0;
+ shells = reallocarray(NULL, nalloc, sizeof (char *));
+ if (shells == NULL)
+ goto fail;
+ while ((cp = fgets(buf, sizeof(buf), fp)) != NULL) {
while (*cp != '#' && *cp != '/' && *cp != '\0')
cp++;
if (*cp == '#' || *cp == '\0')
continue;
- *sp++ = cp;
+ if (!(shells[nshells] = strdup(cp)))
+ goto fail;
+ cp = shells[nshells];
while (!isspace((unsigned char)*cp) && *cp != '#' && *cp !=
'\0')
cp++;
*cp++ = '\0';
+
+ nshells++;
+ if (nshells == nalloc) {
+ char **new = reallocarray(shells, nalloc * 2,
sizeof(char *));
+ if (!new)
+ goto fail;
+ shells = new;
+ nalloc *= 2;
+ }
}
- *sp = NULL;
+ shells[nshells] = NULL;
(void)fclose(fp);
return (shells);
+
+fail:
+ while (nshells)
+ free(shells[nshells--]);
+ free(shells);
+ shells = NULL;
+ (void)fclose(fp);
+ return (okshells);
}
