Fix shell injection vulnerability in patch(1) and drop SCCS

support by replacing system() with execve().

Future revisions may remove the functionality completely.

Obtained from:	Bitrig
Security:	CVE-2015-1416

diff --git a/usr.bin/patch/common.h b/usr.bin/patch/common.h
index 8963c8343a10794184f362e411e46c4abc41875d..f33abcfd79e26fda5b7e6cc7b04291d348ab1bb7 100644 (file)
--- a/usr.bin/patch/common.h
+++ b/usr.bin/patch/common.h
@@ -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"

diff --git a/usr.bin/patch/inp.c b/usr.bin/patch/inp.c
index 826f85b40e9b49f2be3d225491d5b9da9660fae4..e7fa3515f66dc3c9e1950d55b51dc5940fcc9080 100644 (file)
--- a/usr.bin/patch/inp.c
+++ b/usr.bin/patch/inp.c
@@ -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)
-
-               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);
+       (snprintf(lbuf, sizeof(lbuf), f, a1, a2, a3), stat(lbuf, &cstat) == 0)
 
                /*
                 * 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);