patch(1): fix arbitrary code execution bug

This is the combination of two NetBSD patches committed by Christos
Zoulas, based on the findings and Bitrig patch by Martin Natano.
The NetBSD log messages read:

  From Martin Natano @bitrig: Use execve(2) instead of system to
  apply patches that require rcs command execution instead system(3)
  to avoid malicious filenames in patches causing bad things to
  happen. In the process, lose SCCS support. It is not like we are
  shipping sccs commands for that to work.

And:

  Use absolute paths for RCS commands (Martin Natano)

Change-Id: Id44bd59a5a6bc6cd95d1e1fae468bd718cfff2db
This commit is contained in:
David van Moolenbroek 2015-07-26 15:47:05 +00:00
parent 4796287659
commit d8127f841f
2 changed files with 69 additions and 44 deletions

View File

@ -49,8 +49,8 @@
#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"

View File

@ -1,7 +1,7 @@
/*
* $OpenBSD: inp.c,v 1.34 2006/03/11 19:41:30 otto Exp $
* $DragonFly: src/usr.bin/patch/inp.c,v 1.6 2007/09/29 23:11:10 swildner Exp $
* $NetBSD: inp.c,v 1.23 2009/10/21 17:16:11 joerg Exp $
* $NetBSD: inp.c,v 1.24 2015/07/24 18:56:00 christos Exp $
*/
/*
@ -31,14 +31,16 @@
*/
#include <sys/cdefs.h>
__RCSID("$NetBSD: inp.c,v 1.23 2009/10/21 17:16:11 joerg Exp $");
__RCSID("$NetBSD: inp.c,v 1.24 2015/07/24 18:56:00 christos Exp $");
#include <sys/types.h>
#include <sys/file.h>
#include <sys/stat.h>
#include <sys/mman.h>
#include <sys/wait.h>
#include <ctype.h>
#include <errno.h>
#include <fcntl.h>
#include <libgen.h>
#include <limits.h>
@ -139,12 +141,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[MAXLINELEN];
struct stat filestat;
off_t i;
ptrdiff_t sz;
size_t iline, lines_allocated;
pid_t pid;
char *argp[4] = {NULL};
#ifdef DEBUGGING
if (debug & 8)
@ -172,13 +176,13 @@ 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;
@ -186,43 +190,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;
filebase = basename(tmp_filename1);
filedir = dirname(tmp_filename2);
#define try(f, a1, a2, a3) \
(snprintf(s, sizeof lbuf - 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_len, 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_len, 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
@ -230,21 +217,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] = __UNCONST(RCSDIFF);
argp[1] = __UNCONST(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] = __UNCONST(CHECKOUT);
argp[1] = __UNCONST("-l");
argp[2] = __UNCONST(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);