"Jeremy C. Reed" wrote:

> This is some notes from some research. I still need to send-pr these if
> applicable. This is 1.5.1_ALPHA (i386).
> 
> Seg faults with 99999-character long argument.

Here a patch that converts most strcpy()'s to strlcpy()'s (and likewise
for strcat()).  This includes the one OpenBSD change for buffer
overflows.  I've done a bit of testing and don't appear to have broken
anything.  Anyone see any problems before I commit this?

Simon.
--

Index: gzip.c
===================================================================
RCS file: /cvsroot/gnusrc/gnu/usr.bin/gzip/gzip.c,v
retrieving revision 1.7
diff -d -p -u -r1.7 gzip.c
--- gzip.c	2000/11/17 01:31:26	1.7
+++ gzip.c	2001/01/19 04:08:11
@@ -475,7 +475,7 @@ int main (argc, argv)
     }
 #endif
 
-    strncpy(z_suffix, Z_SUFFIX, sizeof(z_suffix)-1);
+    strlcpy(z_suffix, Z_SUFFIX, sizeof(z_suffix));
     z_len = strlen(z_suffix);
 
     while ((optc = getopt_long (argc, argv, "ab:cdfhH?lLmMnNqrS:tvVZ123456789",
@@ -521,7 +521,12 @@ int main (argc, argv)
             if (*optarg == '.') optarg++;
 #endif
             z_len = strlen(optarg);
-            strcpy(z_suffix, optarg);
+	    if (z_len > sizeof(z_suffix)-1) {
+		fprintf(stderr, "%s: -S suffix too long\n", progname);
+		usage();
+		do_exit(ERROR);
+	    }
+            strlcpy(z_suffix, optarg, sizeof(z_suffix));
             break;
 	case 't':
 	    test = decompress = to_stdout = 1;
@@ -635,8 +640,8 @@ local void treat_stdin()
     if (!test && !list && (!decompress || !ascii)) {
 	SET_BINARY_MODE(fileno(stdout));
     }
-    strcpy(ifname, "stdin");
-    strcpy(ofname, "stdout");
+    strlcpy(ifname, "stdin", sizeof(ifname));
+    strlcpy(ofname, "stdout", sizeof(ofname));
 
     /* Get the time stamp on the input file. */
     time_stamp = 0; /* time unknown by default */
@@ -751,7 +756,7 @@ local void treat_file(iname)
      * without a valid gzip suffix (check done in make_ofname).
      */
     if (to_stdout && !list && !test) {
-	strcpy(ofname, "stdout");
+	strlcpy(ofname, "stdout", sizeof(ofname));
 
     } else if (make_ofname() != OK) {
 	return;
@@ -969,9 +974,9 @@ local char *get_suffix(name)
 #endif
     nlen = strlen(name);
     if (nlen <= MAX_SUFFIX+2) {
-        strcpy(suffix, name);
+        strlcpy(suffix, name, sizeof(suffix));
     } else {
-        strcpy(suffix, name+nlen-MAX_SUFFIX-2);
+        strlcpy(suffix, name+nlen-MAX_SUFFIX-2, sizeof(suffix));
     }
     strlwr(suffix);
     slen = strlen(suffix);
@@ -1006,7 +1011,7 @@ local int get_istat(iname, sbuf)
     char *dot; /* pointer to ifname extension, or NULL */
 #endif
 
-    strcpy(ifname, iname);
+    strlcpy(ifname, iname, sizeof(ifname));
 
     /* If input file exists, return OK. */
     if (do_stat(ifname, sbuf) == 0) return OK;
@@ -1028,7 +1033,7 @@ local int get_istat(iname, sbuf)
 #ifdef NO_MULTIPLE_DOTS
     dot = strrchr(ifname, '.');
     if (dot == NULL) {
-        strcat(ifname, ".");
+        strlcat(ifname, ".", sizeof(ifname));
         dot = strrchr(ifname, '.');
     }
 #endif
@@ -1042,24 +1047,26 @@ local int get_istat(iname, sbuf)
         if (*s == '.') s++;
 #endif
 #ifdef MAX_EXT_CHARS
-        strcpy(ifname, iname);
+        strlcpy(ifname, iname, sizeof(ifname));
         /* Needed if the suffixes are not sorted by increasing length */
 
-        if (*dot == '\0') strcpy(dot, ".");
+        if (*dot == '\0')
+	    strlcpy(dot, ".", sizeof(dot));
         dot[MAX_EXT_CHARS+1-strlen(s)] = '\0';
 #endif
-        strcat(ifname, s);
+        strlcat(ifname, s, sizeof(ifname));
         if (do_stat(ifname, sbuf) == 0) return OK;
 	ifname[ilen] = '\0';
     } while (*++suf != NULL);
 
     /* No suffix found, complain using z_suffix: */
 #ifdef MAX_EXT_CHARS
-    strcpy(ifname, iname);
-    if (*dot == '\0') strcpy(dot, ".");
+    strlcpy(ifname, iname, sizeof(ifname));
+    if (*dot == '\0')
+	strlcpy(dot, ".", sizeof(dot));
     dot[MAX_EXT_CHARS+1-z_len] = '\0';
 #endif
-    strcat(ifname, z_suffix);
+    strlcat(ifname, z_suffix, sizeof(ifname));
     perror(ifname);
     exit_code = ERROR;
     return ERROR;
@@ -1073,7 +1080,7 @@ local int make_ofname()
 {
     char *suff;            /* ofname z suffix */
 
-    strcpy(ofname, ifname);
+    strlcpy(ofname, ifname, sizeof(ofname));
     /* strip a version number if any and get the gzip suffix if present: */
     suff = get_suffix(ofname);
 
@@ -1094,7 +1101,7 @@ local int make_ofname()
 	/* Make a special case for .tgz and .taz: */
 	strlwr(suff);
 	if (strequ(suff, ".tgz") || strequ(suff, ".taz")) {
-	    strcpy(suff, ".tar");
+	    strcpy(suff, ".tar");	/* safe */
 	} else {
 	    *suff = '\0'; /* strip the z suffix */
 	}
@@ -1114,10 +1121,10 @@ local int make_ofname()
 #ifdef NO_MULTIPLE_DOTS
 	suff = strrchr(ofname, '.');
 	if (suff == NULL) {
-            strcat(ofname, ".");
+            strlcat(ofname, ".", sizeof(ofname));
 #  ifdef MAX_EXT_CHARS
 	    if (strequ(z_suffix, "z")) {
-		strcat(ofname, "gz"); /* enough room */
+		strlcat(ofname, "gz", sizeof(ofname)); /* enough room */
 		return OK;
 	    }
         /* On the Atari and some versions of MSDOS, name_too_long()
@@ -1130,7 +1137,7 @@ local int make_ofname()
 #  endif
         }
 #endif /* NO_MULTIPLE_DOTS */
-	strcat(ofname, z_suffix);
+	strlcat(ofname, z_suffix, sizeof(ofname));
 
     } /* decompress ? */
     return OK;
@@ -1473,7 +1480,7 @@ local void shorten_name(name)
     len = strlen(name);
     if (decompress) {
 	if (len <= 1) error("name too short");
-	name[len-1] = '\0';
+	name[0] = '\0';
 	return;
     }
     p = get_suffix(name);
@@ -1483,7 +1490,7 @@ local void shorten_name(name)
 
     /* compress 1234567890.tar to 1234567890.tgz */
     if (len > 4 && strequ(p-4, ".tar")) {
-	strcpy(p-4, ".tgz");
+	strcpy(p-4, ".tgz");	/* safe */
 	return;
     }
     /* Try keeping short extensions intact:
@@ -1510,7 +1517,8 @@ local void shorten_name(name)
 	if (trunc == NULL) error("internal error in shorten_name");
 	if (trunc[1] == '\0') trunc--; /* force truncation */
     }
-    strcpy(trunc, z_suffix);
+    /* all arguments to shorten_name are MAX_PATH_LEN long */
+    strlcpy(trunc, z_suffix, MAX_PATH_LEN - (trunc - name));
 }
 
 /* ========================================================================
@@ -1569,7 +1577,7 @@ local int check_ofname()
     /* Ask permission to overwrite the existing file */
     if (!force) {
 	char response[80];
-	strcpy(response,"n");
+	strlcpy(response, "n", sizeof(response));
 	fprintf(stderr, "%s: %s already exists;", progname, ofname);
 	if (foreground && isatty(fileno(stdin))) {
 	    fprintf(stderr, " do you wish to overwrite (y or n)? ");
@@ -1690,7 +1698,7 @@ local void treat_dir(dir)
 	}
 	len = strlen(dir);
 	if (len + NLENGTH(dp) + 1 < MAX_PATH_LEN - 1) {
-	    strcpy(nbuf,dir);
+	    strlcpy(nbuf, dir, sizeof(nbuf));
 	    if (len != 0 /* dir = "" means current dir on Amiga */
 #ifdef PATH_SEP2
 		&& dir[len-1] != PATH_SEP2
@@ -1701,7 +1709,7 @@ local void treat_dir(dir)
 	    ) {
 		nbuf[len++] = PATH_SEP;
 	    }
-	    strcpy(nbuf+len, dp->d_name);
+	    strlcpy(nbuf+len, dp->d_name, sizeof(nbuf) - len);
 	    treat_file(nbuf);
 	} else {
 	    fprintf(stderr,"%s: %s/%s: pathname too long\n",
Index: util.c
===================================================================
RCS file: /cvsroot/gnusrc/gnu/usr.bin/gzip/util.c,v
retrieving revision 1.2
diff -d -p -u -r1.2 util.c
--- util.c	1993/10/15 23:05:56	1.2
+++ util.c	2001/01/19 04:08:13
@@ -291,7 +291,7 @@ char *add_envopt(argcp, argvp, env)
     if (env == NULL) return NULL;
 
     p = (char*)xmalloc(strlen(env)+1);
-    env = strcpy(p, env);                    /* keep env variable intact */
+    env = strcpy(p, env); /* safe */         /* keep env variable intact */
 
     for (p = env; *p; nargc++ ) {            /* move through env */
 	p += strspn(p, SEPARATOR);	     /* skip leading separators */