I. Background The patch(1) utility takes a patch file produced by the diff(1) program and apply the differences to an original file, producing a patched version. The patch(1) utility supports certain version control systems, namely SCCS and RCS, and attempts to get or check out the file before applying a patch, if the original file do not already exist. II. Problem Description Due to insufficient sanitization of the input patch stream, it is possible for a patch file to cause patch(1) to run commands in addition to the desired SCCS or RCS commands. III. Impact This issue could be exploited to execute arbitrary commands as the user invoking patch(1) against a specically crafted patch file, which could be leveraged to obtain elevated privileges. Index: usr.bin/patch/common.h =================================================================== --- usr.bin/patch/common.h (revision 285926) +++ usr.bin/patch/common.h (working copy) @@ -43,12 +43,10 @@ #define LINENUM_MAX LONG_MAX #define SCCSPREFIX "s." -#define GET "get -e %s" -#define SCCSDIFF "get -p %s | diff - %s >/dev/null" #define RCSSUFFIX ",v" -#define CHECKOUT "co -l %s" -#define RCSDIFF "rcsdiff %s > /dev/null" +#define CHECKOUT "/usr/bin/co" +#define RCSDIFF "/usr/bin/rcsdiff" #define ORIGEXT ".orig" #define REJEXT ".rej" Index: usr.bin/patch/inp.c =================================================================== --- usr.bin/patch/inp.c (revision 285926) +++ usr.bin/patch/inp.c (working copy) @@ -31,8 +31,10 @@ #include <sys/file.h> #include <sys/stat.h> #include <sys/mman.h> +#include <sys/wait.h> #include <ctype.h> +#include <errno.h> #include <libgen.h> #include <stddef.h> #include <stdint.h> @@ -133,12 +135,14 @@ reallocate_lines(size_t *lines_allocated) static bool plan_a(const char *filename) { - int ifd, statfailed; + int ifd, statfailed, devnull, pstat; char *p, *s, lbuf[INITLINELEN]; struct stat filestat; ptrdiff_t sz; size_t i; size_t iline, lines_allocated; + pid_t pid; + char *argp[4] = {NULL}; #ifdef DEBUGGING if (debug & 8) @@ -166,13 +170,14 @@ plan_a(const char *filename) } if (statfailed && check_only) fatal("%s not found, -C mode, can't probe further\n", filename); - /* For nonexistent or read-only files, look for RCS or SCCS versions. */ + /* For nonexistent or read-only files, look for RCS versions. */ + if (statfailed || /* No one can write to it. */ (filestat.st_mode & 0222) == 0 || /* I can't write to it. */ ((filestat.st_mode & 0022) == 0 && filestat.st_uid != getuid())) { - const char *cs = NULL, *filebase, *filedir; + char *filebase, *filedir; struct stat cstat; char *tmp_filename1, *tmp_filename2; @@ -180,43 +185,26 @@ plan_a(const char *filename) tmp_filename2 = strdup(filename); if (tmp_filename1 == NULL || tmp_filename2 == NULL) fatal("strdupping filename"); + filebase = basename(tmp_filename1); filedir = dirname(tmp_filename2); - /* Leave room in lbuf for the diff command. */ - s = lbuf + 20; - #define try(f, a1, a2, a3) \ - (snprintf(s, buf_size - 20, f, a1, a2, a3), stat(s, &cstat) == 0) + (snprintf(lbuf, sizeof(lbuf), f, a1, a2, a3), stat(lbuf, &cstat) == 0) - if (try("%s/RCS/%s%s", filedir, filebase, RCSSUFFIX) || - try("%s/RCS/%s%s", filedir, filebase, "") || - try("%s/%s%s", filedir, filebase, RCSSUFFIX)) { - snprintf(buf, buf_size, CHECKOUT, filename); - snprintf(lbuf, sizeof lbuf, RCSDIFF, filename); - cs = "RCS"; - } else if (try("%s/SCCS/%s%s", filedir, SCCSPREFIX, filebase) || - try("%s/%s%s", filedir, SCCSPREFIX, filebase)) { - snprintf(buf, buf_size, GET, s); - snprintf(lbuf, sizeof lbuf, SCCSDIFF, s, filename); - cs = "SCCS"; - } else if (statfailed) - fatal("can't find %s\n", filename); - - free(tmp_filename1); - free(tmp_filename2); - /* * else we can't write to it but it's not under a version * control system, so just proceed. */ - if (cs) { + if (try("%s/RCS/%s%s", filedir, filebase, RCSSUFFIX) || + try("%s/RCS/%s%s", filedir, filebase, "") || + try("%s/%s%s", filedir, filebase, RCSSUFFIX)) { if (!statfailed) { if ((filestat.st_mode & 0222) != 0) /* The owner can write to it. */ fatal("file %s seems to be locked " - "by somebody else under %s\n", - filename, cs); + "by somebody else under RCS\n", + filename); /* * It might be checked out unlocked. See if * it's safe to check out the default version @@ -224,21 +212,59 @@ plan_a(const char *filename) */ if (verbose) say("Comparing file %s to default " - "%s version...\n", - filename, cs); - if (system(lbuf)) + "RCS version...\n", filename); + + switch (pid = fork()) { + case -1: + fatal("can't fork: %s\n", + strerror(errno)); + case 0: + devnull = open("/dev/null", O_RDONLY); + if (devnull == -1) { + fatal("can't open /dev/null: %s", + strerror(errno)); + } + (void)dup2(devnull, STDOUT_FILENO); + argp[0] = strdup(RCSDIFF); + argp[1] = strdup(filename); + execv(RCSDIFF, argp); + exit(127); + } + pid = waitpid(pid, &pstat, 0); + if (pid == -1 || WEXITSTATUS(pstat) != 0) { fatal("can't check out file %s: " - "differs from default %s version\n", - filename, cs); + "differs from default RCS version\n", + filename); + } } + if (verbose) - say("Checking out file %s from %s...\n", - filename, cs); - if (system(buf) || stat(filename, &filestat)) - fatal("can't check out file %s from %s\n", - filename, cs); + say("Checking out file %s from RCS...\n", + filename); + + switch (pid = fork()) { + case -1: + fatal("can't fork: %s\n", strerror(errno)); + case 0: + argp[0] = strdup(CHECKOUT); + argp[1] = strdup("-l"); + argp[2] = strdup(filename); + execv(CHECKOUT, argp); + exit(127); + } + pid = waitpid(pid, &pstat, 0); + if (pid == -1 || WEXITSTATUS(pstat) != 0 || + stat(filename, &filestat)) { + fatal("can't check out file %s from RCS\n", + filename); + } + } else if (statfailed) { + fatal("can't find %s\n", filename); } + free(tmp_filename1); + free(tmp_filename2); } + filemode = filestat.st_mode; if (!S_ISREG(filemode)) fatal("%s is not a normal file--can't patch\n", filename);