Re: user/6573: /usr/bin/gzip with long argument list core-dumps

[Todd C. Miller]

The following reply was made to PR user/6573; it has been noted by GNATS.

From: "Todd C. Miller" <todd.mil...@courtesan.com>
To: Jonathan Thornburg <jth...@astro.indiana.edu>
Cc: gn...@openbsd.org
Subject: Re: user/6573: /usr/bin/gzip with long argument list core-dumps
Date: Fri, 04 Mar 2011 07:42:51 -0500

 The following diff fixes the bug.
 
  - todd
 
 Index: usr.bin/compress/main.c
 ===================================================================
 RCS file: /home/cvs/openbsd/src/usr.bin/compress/main.c,v
 retrieving revision 1.76
 diff -u -r1.76 main.c
 --- usr.bin/compress/main.c    28 Jul 2010 23:52:01 -0000      1.76
 +++ usr.bin/compress/main.c    3 Mar 2011 23:18:57 -0000
 @@ -138,8 +138,7 @@
        const char *s;
        char *p, *infile;
        char outfile[MAXPATHLEN], _infile[MAXPATHLEN], suffix[16];
 -      char *nargv[512];       /* some estimate based on ARG_MAX */
 -      int bits, ch, error, i, rc, cflag, oflag;
 +      int bits, ch, error, rc, cflag, oflag;
        static const char *optstr[3] = {
                "123456789ab:cdfghLlNnOo:qrS:tVv",
                "cfhlNno:qrtVv",
 @@ -180,21 +179,30 @@
  
        strlcpy(suffix, method->suffix, sizeof(suffix));
  
 -      nargv[0] = NULL;
        if (method == M_DEFLATE && (p = getenv("GZIP")) != NULL) {
 -              char *last;
 +              char *evbuf, *last, **nargv = NULL;
 +              int argc_extra = 0, nargc = 0;
  
 -              nargv[0] = *argv++;
 -              for (i = 1, (p = strtok_r(p, " ", &last)); p != NULL;
 -                  (p = strtok_r(NULL, " ", &last)), i++)
 -                      if (i < sizeof(nargv)/sizeof(nargv[1]) - argc - 1)
 -                              nargv[i] = p;
 -                      else
 -                              errx(1, "GZIP is too long");
 -              argc += i - 1;
 -              while ((nargv[i++] = *argv++))
 -                      ;
 -              argv = nargv;
 +              if ((evbuf = strdup(p)) == NULL)
 +                      err(1, NULL);
 +              for ((p = strtok_r(evbuf, " ", &last)); p != NULL;
 +                  (p = strtok_r(NULL, " ", &last))) {
 +                      if (nargc + 1 >= argc_extra) {
 +                              argc_extra += 1024;
 +                              nargv = realloc(nargv,
 +                                  (argc + argc_extra + 1) * sizeof(char *));
 +                              if (nargv == NULL)
 +                                      err(1, NULL);
 +                      }
 +                      nargv[++nargc] = p;
 +              }
 +              if (nargv != NULL) {
 +                      nargv[0] = *argv++;
 +                      while ((nargv[++nargc] = *argv++))
 +                              ;
 +                      argv = nargv;
 +                      argc = nargc;
 +              }
        }
  
        while ((ch = getopt_long(argc, argv, optstr[pmode], longopts, NULL)) != 
-1)
 @@ -305,11 +313,11 @@
        argv += optind;
  
        if (argc == 0) {
 -              if (nargv[0] == NULL)
 -                      argv = nargv;
 -              /* XXX - make sure we don't oflow nargv in $GZIP case (millert) 
*/
 +              argv = calloc(2, sizeof(char *));
 +              if (argv == NULL)
 +                      err(1, NULL);
                argv[0] = "-";
 -              argv[1] = NULL;
 +              argc = 1;
        }
        if (oflag && (recurse || argc > 1))
                errx(1, "-o option may only be used with a single input file");