From owner-svn-src-all@freebsd.org Tue Jul 28 19:58:37 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 9CD0E9AD1E8; Tue, 28 Jul 2015 19:58:37 +0000 (UTC) (envelope-from delphij@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8D10791D; Tue, 28 Jul 2015 19:58:37 +0000 (UTC) (envelope-from delphij@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.70]) by repo.freebsd.org (8.14.9/8.14.9) with ESMTP id t6SJwbTX002399; Tue, 28 Jul 2015 19:58:37 GMT (envelope-from delphij@FreeBSD.org) Received: (from delphij@localhost) by repo.freebsd.org (8.14.9/8.14.9/Submit) id t6SJwbT0002397; Tue, 28 Jul 2015 19:58:37 GMT (envelope-from delphij@FreeBSD.org) Message-Id: <201507281958.t6SJwbT0002397@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: delphij set sender to delphij@FreeBSD.org using -f From: Xin LI Date: Tue, 28 Jul 2015 19:58:37 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r285974 - head/usr.bin/patch X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Jul 2015 19:58:37 -0000 Author: delphij Date: Tue Jul 28 19:58:36 2015 New Revision: 285974 URL: https://svnweb.freebsd.org/changeset/base/285974 Log: 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 Modified: head/usr.bin/patch/common.h head/usr.bin/patch/inp.c Modified: head/usr.bin/patch/common.h ============================================================================== --- head/usr.bin/patch/common.h Tue Jul 28 19:15:44 2015 (r285973) +++ head/usr.bin/patch/common.h Tue Jul 28 19:58:36 2015 (r285974) @@ -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" Modified: head/usr.bin/patch/inp.c ============================================================================== --- head/usr.bin/patch/inp.c Tue Jul 28 19:15:44 2015 (r285973) +++ head/usr.bin/patch/inp.c Tue Jul 28 19:58:36 2015 (r285974) @@ -31,8 +31,10 @@ #include #include #include +#include #include +#include #include #include #include @@ -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);