Date: Sun, 2 Jul 2000 07:40:14 -0700 (PDT) From: kbyanc@posi.net To: FreeBSD-gnats-submit@freebsd.org Subject: bin/19642: patch to merge OpenBSD changes to patch(1) Message-ID: <200007021440.HAA70176@kbyanc.corp.ONElist.com>
next in thread | raw e-mail | index | archive | help
>Number: 19642 >Category: bin >Synopsis: patch to merge OpenBSD changes to patch(1) >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: change-request >Submitter-Id: current-users >Arrival-Date: Sun Jul 02 07:50:01 PDT 2000 >Closed-Date: >Last-Modified: >Originator: Kelly Yancey >Release: FreeBSD 5.0-CURRENT i386 >Organization: eGroups.com -- http://www.eGroups.com/ >Environment: FreeBSD backroom.corp.ONElist.com 5.0-CURRENT FreeBSD 5.0-CURRENT #1: Fri Jun 30 13:47:43 PDT 2000 root@backroom.corp.ONElist.com:/usr/src/sys/compile/BACKROOM i386 >Description: Not surprisingly, OpenBSD has fixed a number of potential security holes in patch(1), this patch merges those changes into FreeBSD's patch(1). Specifically, the attached patch catches a number of buffer overflow cases as well as the standard mktemp race conditions. Beyond just security, OpenBSD has also generally cleaned up the code a bit, which changes I have also included. Basically, this just entails replacing magic numbers with #defines and answering -Wall's warnings (note, however -Wall still complains about a few non-issues). As a followup to this patch, with permission, I would like to also remove all the obsolete $Log entries at the tops of the source files. Finally, for the record, I am somewhat confused about the presence of contrib/patch also. The version in contrib is a much newer (albeit entirely GPL'ed) version of patch. However, as best I can tell it is completely unreferenced. The version of patch in gnu/usr.bin/patch (the version this diff was taken against) is the version installed as part of `make world`. It would appear that the copy in contrib/patch is superfluous. -Kelly >How-To-Repeat: >Fix: Index: gnu/usr.bin/patch/backupfile.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/backupfile.c,v retrieving revision 1.4 diff -u -r1.4 backupfile.c --- gnu/usr.bin/patch/backupfile.c 1998/01/21 14:37:14 1.4 +++ gnu/usr.bin/patch/backupfile.c 2000/07/02 13:00:16 @@ -216,7 +216,7 @@ char *str1, *str2; { char *newstr; - char str1_length = strlen (str1); + int str1_length = strlen (str1); newstr = malloc (str1_length + strlen (str2) + 1); if (newstr == 0) Index: gnu/usr.bin/patch/common.h =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/common.h,v retrieving revision 1.7 diff -u -r1.7 common.h --- gnu/usr.bin/patch/common.h 1999/09/05 17:31:55 1.7 +++ gnu/usr.bin/patch/common.h 2000/07/02 14:13:22 @@ -29,8 +29,9 @@ #define Fclose (void)fclose #define Fflush (void)fflush #define Sprintf (void)sprintf -#define Mktemp (void)mktemp +#define Snprintf (void)snprintf #define Strcpy (void)strcpy +#define Strlcpy (void)strlcpy #define Strcat (void)strcat /* NeXT declares malloc and realloc incompatibly from us in some of @@ -41,6 +42,7 @@ #include <assert.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/param.h> #include <ctype.h> #include <signal.h> #undef malloc @@ -117,8 +119,9 @@ EXT bool ok_to_create_file INIT(FALSE); EXT char *bestguess INIT(Nullch); /* guess at correct filename */ +#define REJNAMESIZ 128 EXT char *outname INIT(Nullch); -EXT char rejname[128]; +EXT char rejname[REJNAMESIZ]; EXT char *origprae INIT(Nullch); @@ -150,11 +153,12 @@ #define UNI_DIFF 5 EXT int diff_type INIT(0); +#define DEFSIZ 128 EXT bool do_defines INIT(FALSE); /* patch using ifdef, ifndef, etc. */ -EXT char if_defined[128]; /* #ifdef xyzzy */ -EXT char not_defined[128]; /* #ifndef xyzzy */ +EXT char if_defined[DEFSIZ]; /* #ifdef xyzzy */ +EXT char not_defined[DEFSIZ]; /* #ifndef xyzzy */ EXT char else_defined[] INIT("#else\n");/* #else */ -EXT char end_defined[128]; /* #endif xyzzy */ +EXT char end_defined[DEFSIZ]; /* #endif xyzzy */ EXT char *revision INIT(Nullch); /* prerequisite revision, if any */ @@ -172,7 +176,7 @@ char *strcpy(); char *strcat(); #endif -char *mktemp(); +int mkstemp(); #ifdef HAVE_UNISTD_H #include <unistd.h> #else Index: gnu/usr.bin/patch/getopt.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/getopt.c,v retrieving revision 1.4 diff -u -r1.4 getopt.c --- gnu/usr.bin/patch/getopt.c 1998/01/21 14:37:18 1.4 +++ gnu/usr.bin/patch/getopt.c 2000/07/02 14:14:11 @@ -445,7 +445,7 @@ int exact = 0; int ambig = 0; const struct option *pfound = NULL; - int indfound; + int indfound = 0; while (*s && *s != '=') s++; Index: gnu/usr.bin/patch/inp.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/inp.c,v retrieving revision 1.10 diff -u -r1.10 inp.c --- gnu/usr.bin/patch/inp.c 1999/09/05 17:31:55 1.10 +++ gnu/usr.bin/patch/inp.c 2000/07/02 14:05:44 @@ -18,7 +18,7 @@ /* Input-file-with-indexable-lines abstract type */ -static long i_size; /* size of the input file */ +static off_t i_size; /* size of the input file */ static char *i_womp; /* plan a buffer for entire file */ static char **i_ptr; /* pointers to lines in i_womp */ @@ -117,7 +117,9 @@ s = lbuf + 20; strncpy(s, filename, pathlen); -#define try(f,a1,a2) (Sprintf(s + pathlen, f, a1, a2), stat(s, &cstat) == 0) +#define try(f,a1,a2) \ + (Snprintf(s + pathlen, sizeof lbuf - (s + pathlen - lbuf), \ + f, a1, a2), stat(s, &cstat) == 0) if (( try("RCS/%s%s", filebase, RCSSUFFIX) || try("RCS/%s%s", filebase, "") || try( "%s%s", filebase, RCSSUFFIX)) @@ -127,13 +129,14 @@ (statfailed || ( (filestat.st_dev ^ cstat.st_dev) | (filestat.st_ino ^ cstat.st_ino)))) { - Sprintf(buf, output_elsewhere?CHECKOUT:CHECKOUT_LOCKED, filename); - Sprintf(lbuf, RCSDIFF, filename); + Snprintf(buf, sizeof(buf), + output_elsewhere?CHECKOUT:CHECKOUT_LOCKED, filename); + Snprintf(lbuf, sizeof(lbuf), RCSDIFF, filename); cs = "RCS"; } else if ( try("SCCS/%s%s", SCCSPREFIX, filebase) || try( "%s%s", SCCSPREFIX, filebase)) { - Sprintf(buf, output_elsewhere?GET:GET_LOCKED, s); - Sprintf(lbuf, SCCSDIFF, s, filename); + Snprintf(buf, sizeof(buf), output_elsewhere?GET:GET_LOCKED, s); + Snprintf(lbuf, sizeof(lbuf), SCCSDIFF, s, filename); cs = "SCCS"; } else if (statfailed) fatal2("can't find %s\n", filename); @@ -172,15 +175,16 @@ #ifdef lint i_womp = Nullch; #else - i_womp = malloc((MEM)(i_size+2)); /* lint says this may alloc less than */ + i_womp = (char *)malloc((MEM)(i_size+2)); + /* lint says this may alloc less than */ /* i_size, but that's okay, I think. */ #endif if (i_womp == Nullch) return FALSE; - if ((ifd = open(filename, 0)) < 0) + if ((ifd = open(filename, O_RDONLY)) < 0) pfatal2("can't open file %s", filename); #ifndef lint - if (read(ifd, i_womp, (int)i_size) != i_size) { + if (read(ifd, i_womp, (size_t)i_size) != i_size) { Close(ifd); /* probably means i_size > 15 or 16 bits worth */ free(i_womp); /* at this point it doesn't matter if i_womp was */ return FALSE; /* undersized. */ @@ -296,8 +300,8 @@ Fseek(ifp, 0L, 0); /* rewind file */ lines_per_buf = BUFFERSIZE / maxlen; tireclen = maxlen; - tibuf[0] = malloc((MEM)(BUFFERSIZE + 1)); - tibuf[1] = malloc((MEM)(BUFFERSIZE + 1)); + tibuf[0] = (char *)malloc((MEM)(BUFFERSIZE + 1)); + tibuf[1] = (char *)malloc((MEM)(BUFFERSIZE + 1)); if (tibuf[1] == Nullch) fatal1("out of memory\n"); for (i=1; ; i++) { @@ -315,7 +319,7 @@ } Fclose(ifp); Close(tifd); - if ((tifd = open(TMPINNAME, 0)) < 0) { + if ((tifd = open(TMPINNAME, O_RDONLY)) < 0) { pfatal2("can't reopen file %s", TMPINNAME); } } @@ -342,7 +346,7 @@ else { tiline[whichbuf] = baseline; #ifndef lint /* complains of long accuracy */ - Lseek(tifd, (long)baseline / lines_per_buf * BUFFERSIZE, 0); + Lseek(tifd, (off_t)baseline / lines_per_buf * BUFFERSIZE, 0); #endif if (read(tifd, tibuf[whichbuf], BUFFERSIZE) < 0) pfatal2("error reading tmp file %s", TMPINNAME); Index: gnu/usr.bin/patch/patch.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/patch.c,v retrieving revision 1.16 diff -u -r1.16 patch.c --- gnu/usr.bin/patch/patch.c 1999/09/05 17:31:55 1.16 +++ gnu/usr.bin/patch/patch.c 2000/07/02 13:10:48 @@ -115,7 +115,11 @@ bool patch_match(); bool similar(); void re_input(); +#ifdef __GNUC__ +void my_exit() __attribute__((noreturn)); +#else void my_exit(); +#endif /* TRUE if -E was specified on command line. */ static int remove_empty_files = FALSE; @@ -170,22 +174,31 @@ TMPOUTNAME = (char *) malloc (tmpname_len); strcpy (TMPOUTNAME, tmpdir); strcat (TMPOUTNAME, "/patchoXXXXXX"); - Mktemp(TMPOUTNAME); + if ((i = mkstemp(TMPOUTNAME)) < 0) + pfatal2("can't create %s", TMPOUTNAME); + Close(i); TMPINNAME = (char *) malloc (tmpname_len); strcpy (TMPINNAME, tmpdir); strcat (TMPINNAME, "/patchiXXXXXX"); - Mktemp(TMPINNAME); + if ((i = mkstemp(TMPINNAME)) < 0) + pfatal2("can't create %s", TMPINNAME); + Close(i); TMPREJNAME = (char *) malloc (tmpname_len); strcpy (TMPREJNAME, tmpdir); strcat (TMPREJNAME, "/patchrXXXXXX"); - Mktemp(TMPREJNAME); + if ((i = mkstemp(TMPREJNAME)) < 0) + pfatal2("can't create %s", TMPREJNAME); + Close(i); TMPPATNAME = (char *) malloc (tmpname_len); strcpy (TMPPATNAME, tmpdir); strcat (TMPPATNAME, "/patchpXXXXXX"); - Mktemp(TMPPATNAME); + if ((i = mkstemp(TMPPATNAME)) < 0) + pfatal2("can't create %s", TMPPATNAME); + Close(i); + } { @@ -383,7 +396,9 @@ if (failed) { failtotal += failed; if (!*rejname) { - Strcpy(rejname, outname); + if (strlcpy(rejname, outname, sizeof(rejname) - sizeof(".rej")) + >= sizeof(rejname) - sizeof(".rej")) + fatal2("filename %s is too long\n", outname); addext(rejname, ".rej", '#'); } if (skip_rest_of_patch) { @@ -519,9 +534,11 @@ do_defines = TRUE; if (!isalpha((unsigned char)*optarg) && '_' != *optarg) fatal1("argument to -D is not an identifier\n"); - Sprintf(if_defined, "#ifdef %s\n", optarg); - Sprintf(not_defined, "#ifndef %s\n", optarg); - Sprintf(end_defined, "#endif /* %s */\n", optarg); + Snprintf(if_defined, sizeof if_defined, "#ifdef %s\n", optarg); + Snprintf(not_defined, sizeof not_defined, "#ifndef %s\n", + optarg); + Snprintf(end_defined, sizeof end_defined, "#endif /* %s */\n", + optarg); break; case 'e': diff_type = ED_DIFF; @@ -557,7 +574,7 @@ strippath = 0; break; case 'r': - Strcpy(rejname, optarg); + Strlcpy(rejname, optarg, sizeof(rejname)); break; case 'R': reverse = TRUE; Index: gnu/usr.bin/patch/pch.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/pch.c,v retrieving revision 1.16 diff -u -r1.16 pch.c --- gnu/usr.bin/patch/pch.c 1999/09/05 17:31:55 1.16 +++ gnu/usr.bin/patch/pch.c 2000/07/02 14:11:19 @@ -1051,8 +1051,8 @@ else indent++; } - if (buf != s) - Strcpy(buf, s); + if (buf != s && strlcpy(buf, s, sizeof(buf)) >= sizeof(buf)) + fatal1("buffer too small in pgets()\n"); } return ret; } Index: gnu/usr.bin/patch/util.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/util.c,v retrieving revision 1.7 diff -u -r1.7 util.c --- gnu/usr.bin/patch/util.c 1998/01/21 14:37:25 1.7 +++ gnu/usr.bin/patch/util.c 2000/07/02 13:44:48 @@ -4,7 +4,11 @@ #include "util.h" #include "backupfile.h" +#ifdef __GNUC__ +void my_exit() __attribute__((noreturn)); +#else void my_exit(); +#endif #ifndef HAVE_STRERROR static char * @@ -27,7 +31,7 @@ move_file(from,to) char *from, *to; { - char bakname[512]; + char bakname[MAXPATHLEN]; Reg1 char *s; Reg2 int i; Reg3 int fromfd; @@ -39,7 +43,7 @@ if (debug & 4) say2("Moving %s to stdout.\n", from); #endif - fromfd = open(from, 0); + fromfd = open(from, O_RDONLY); if (fromfd < 0) pfatal2("internal error, can't reopen %s", from); while ((i=read(fromfd, buf, sizeof buf)) > 0) @@ -50,14 +54,16 @@ } if (origprae) { - Strcpy(bakname, origprae); - Strcat(bakname, to); + if (strlcpy(bakname, origprae, sizeof(bakname)) >= sizeof(bakname) || + strlcat(bakname, to, sizeof(bakname)) >= sizeof(bakname)) + fatal2("filename %s too long for buffer\n", origprae); } else { #ifndef NODIR char *backupname = find_backup_file_name(to); if (backupname == (char *) 0) fatal1("out of memory\n"); - Strcpy(bakname, backupname); + if (strlcpy(bakname, backupname, sizeof(bakname)) >= sizeof(bakname)) + fatal2("filename %s too long for buffer\n", backupname); free(backupname); #else /* NODIR */ Strcpy(bakname, to); @@ -111,7 +117,7 @@ to, from, strerror(errno)); return -1; } - fromfd = open(from, 0); + fromfd = open(from, O_RDONLY); if (fromfd < 0) pfatal2("internal error, can't reopen %s", from); while ((i=read(fromfd, buf, sizeof buf)) > 0) @@ -137,7 +143,7 @@ tofd = creat(to, 0666); if (tofd < 0) pfatal2("can't create %s", to); - fromfd = open(from, 0); + fromfd = open(from, O_RDONLY); if (fromfd < 0) pfatal2("internal error, can't reopen %s", from); while ((i=read(fromfd, buf, sizeof buf)) > 0) @@ -160,7 +166,7 @@ s = "Oops"; t = s; while (*t++); - rv = malloc((MEM) (t - s)); + rv = (char *)malloc((MEM) (t - s)); if (rv == Nullch) { if (using_plan_a) out_of_mem = TRUE; @@ -236,7 +242,7 @@ int r; bool tty2 = isatty(2); - Sprintf(buf, pat, arg1, arg2, arg3); + Snprintf(buf, sizeof buf, pat, arg1, arg2, arg3); Fflush(stderr); write(2, buf, strlen(buf)); if (tty2) { /* might be redirected to a file */ @@ -247,7 +253,7 @@ write(1, buf, strlen(buf)); r = read(1, buf, sizeof buf); } - else if ((ttyfd = open("/dev/tty", 2)) >= 0 && isatty(ttyfd)) { + else if ((ttyfd = open("/dev/tty", O_RDWR)) >= 0 && isatty(ttyfd)) { /* might be deleted or unwriteable */ write(ttyfd, buf, strlen(buf)); r = read(ttyfd, buf, sizeof buf); @@ -319,45 +325,25 @@ Reg1 char *filename; bool striplast; { - char tmpbuf[256]; - Reg2 char *s = tmpbuf; - char *dirv[20]; /* Point to the NULs between elements. */ - Reg3 int i; - Reg4 int dirvp = 0; /* Number of finished entries in dirv. */ - - /* Copy `filename' into `tmpbuf' with a NUL instead of a slash - between the directories. */ - while (*filename) { - if (*filename == '/') { - filename++; - dirv[dirvp++] = s; - *s++ = '\0'; - } - else { - *s++ = *filename++; - } - } - *s = '\0'; - dirv[dirvp] = s; - if (striplast) - dirvp--; - if (dirvp < 0) - return; - - strcpy(buf, "mkdir"); - s = buf; - for (i=0; i<=dirvp; i++) { - struct stat sbuf; - - if (stat(tmpbuf, &sbuf) && errno == ENOENT) { - while (*s) s++; - *s++ = ' '; - strcpy(s, tmpbuf); - } - *dirv[i] = '/'; + + char *tmpbuf; + + if ((tmpbuf = strdup(filename)) == NULL) + fatal1("out of memory\n"); + + if (striplast) { + char *s = strrchr(tmpbuf, '/'); + if (s == NULL) + return; /* nothing to be done */ + *s = '\0'; } - if (s != buf) - system(buf); + + strcpy(buf, "/bin/mkdir -p "); + if (strlcat(buf, tmpbuf, sizeof(buf)) >= sizeof(buf)) + fatal2("buffer too small to hold %.20s...\n", tmpbuf); + + if (system(buf)) + pfatal2("%.40s failed", buf); } /* Make filenames more reasonable. */ @@ -374,7 +360,7 @@ char tmpbuf[200]; int sleading = strip_leading; - if (!at) + if (!at || *at == '\0') return Nullch; while (isspace((unsigned char)*at)) at++; @@ -410,17 +396,15 @@ if (stat(name, &filestat) && !assume_exists) { char *filebase = basename(name); - int pathlen = filebase - name; - - /* Put any leading path into `tmpbuf'. */ - strncpy(tmpbuf, name, pathlen); + char *filedir = dirname(name); -#define try(f, a1, a2) (Sprintf(tmpbuf + pathlen, f, a1, a2), stat(tmpbuf, &filestat) == 0) - if ( try("RCS/%s%s", filebase, RCSSUFFIX) - || try("RCS/%s%s", filebase, "") - || try( "%s%s", filebase, RCSSUFFIX) - || try("SCCS/%s%s", SCCSPREFIX, filebase) - || try( "%s%s", SCCSPREFIX, filebase)) +#define try(f, a1, a2, a3) (Snprintf(tmpbuf, sizeof(tmpbuf), f, a1, a2, a3), \ + stat(tmpbuf, &filestat) == 0) + if ( try("%s/RCS/%s%s", filedir, filebase, RCSSUFFIX) + || try("%s/RCS/%s%s", filedir, filebase, "") + || try( "%s/%s%s", filedir, filebase, RCSSUFFIX) + || try("%s/SCCS/%s%s", filedir, SCCSPREFIX, filebase) + || try( "%s/%s%s", filedir, SCCSPREFIX, filebase)) return name; free(name); name = Nullch; Index: gnu/usr.bin/patch/util.h =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/util.h,v retrieving revision 1.7 diff -u -r1.7 util.h --- gnu/usr.bin/patch/util.h 1999/09/05 17:31:55 1.7 +++ gnu/usr.bin/patch/util.h 2000/07/02 13:52:55 @@ -86,3 +86,5 @@ void ignore_signals(); void makedirs(); char *basename(); +char *dirname(); + Index: gnu/usr.bin/patch/version.c =================================================================== RCS file: /home/cvs/src/gnu/usr.bin/patch/version.c,v retrieving revision 1.6 diff -u -r1.6 version.c --- gnu/usr.bin/patch/version.c 1999/09/05 17:31:55 1.6 +++ gnu/usr.bin/patch/version.c 2000/07/02 13:54:27 @@ -13,7 +13,11 @@ #include "patchlevel.h" #include "version.h" +#ifdef __GNUC__ +void my_exit() __attribute__((noreturn)); +#else void my_exit(); +#endif /* Print out the version number and die. */ >Release-Note: >Audit-Trail: >Unformatted: To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200007021440.HAA70176>