Hi,
the code for temporary file handling in patch is currently rather poor,
leaving possibilities for race conditions while patching files. Granted,
there is a bug in patch that makes it rather hard to be successfully
exploited as long as /tmp is on its own partition (which is basically
always true, I hope). Also permissions of the plan b buffer file are
changed from 600 to 644 (i.e. 666 + umask)
Beside of that, patch's output isn't always true when it comes to
rejected files that couldn't be saved as "*.rej", i.e. when they are
left in temporary directory.
I'll try to explain this by using this example:
$ cat my.diff
--- a Sat Dec 13 19:28:53 2014
+++ a~ Sat Dec 13 19:28:58 2014
@@ -1,3 +1,3 @@
1
-a
+b
2
--- b Sat Dec 13 19:29:03 2014
+++ b~ Sat Dec 13 19:29:07 2014
@@ -1,3 +1,3 @@
2
-a
+c
3
--- c Sat Dec 13 20:43:30 2014
+++ c~ Sat Dec 13 20:43:35 2014
@@ -0,0 +1 @@
+c
$ touch a b c
$ sudo mkdir a.rej b.rej
$ patch -i my.diff
Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- a Sat Dec 13 19:28:53 2014
|+++ a~ Sat Dec 13 19:28:58 2014
--------------------------
Patching file a using Plan A...
Hunk #1 failed at 1.
1 out of 1 hunks failed--saving rejects to a.rej
Can't backup a.rej, output is in /tmp/patchr4mBV12Ow1u: Permission denied
Hmm... The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- b Sat Dec 13 19:29:03 2014
|+++ b~ Sat Dec 13 19:29:07 2014
--------------------------
Patching file b using Plan A...
Hunk #1 failed at 1.
1 out of 1 hunks failed--saving rejects to b.rej
Can't backup b.rej, output is in /tmp/patchr4mBV12Ow1u: Permission denied
Hmm... The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- c Sat Dec 13 20:43:30 2014
|+++ c~ Sat Dec 13 20:43:35 2014
--------------------------
Patching file c using Plan A...
Empty context always matches.
Hunk #1 succeeded at 1.
done
$ cat /tmp/patchr4mBV12Ow1u
$ _
As you can see, the temporary file is empty. Even if it would be not
empty, it would contain the rejected hunks from files "a" and "b".
The file is empty because the last patch operation on file "c" was
successful. Before each file operation, these temporary files are
re-opened; without calling mkstemp() or the like.
The race condition happens if /tmp is on the same partition as the
target files. Could also be triggered if TMPDIR environmental variable
is adjusted. After successful operations, the temporary file that
contains the output is rename()'ed, effectively giving an attacker
a time window to put a new file into the old position -- he would
even know the file name to use...
My diff unifies temporary file handling into temp.c. Also, it avoids
to re-open files by using proper permissions. In pch.c, it means that
the temporary patch file is opened "r+" to be filled with stdin. In
inp.c, the buffer file for plan b is kept open as "r+", too...
To test various cases, these calls are of interest:
$ patch -i my.diff # least file operations
$ patch -x 8 -i my.diff # uses TMP_IN for plan b buffer
$ patch < my.diff # uses TMP_PAT for stdin buffer
$ patch -x 8 < my.diff # most file operations
Thoughts on this?
Tobias
Index: Makefile
===================================================================
RCS file: /cvs/src/usr.bin/patch/Makefile,v
retrieving revision 1.4
diff -u -p -u -p -r1.4 Makefile
--- Makefile 16 May 2005 15:22:46 -0000 1.4
+++ Makefile 13 Dec 2014 19:38:19 -0000
@@ -1,6 +1,6 @@
# $OpenBSD: Makefile,v 1.4 2005/05/16 15:22:46 espie Exp $
PROG= patch
-SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c
+SRCS= patch.c pch.c inp.c util.c backupfile.c mkpath.c temp.c
.include <bsd.prog.mk>
Index: common.h
===================================================================
RCS file: /cvs/src/usr.bin/patch/common.h,v
retrieving revision 1.28
diff -u -p -u -p -r1.28 common.h
--- common.h 25 Nov 2014 10:26:07 -0000 1.28
+++ common.h 13 Dec 2014 19:38:19 -0000
@@ -48,6 +48,11 @@
#define ORIGEXT ".orig"
#define REJEXT ".rej"
+#define TMP_OUT 0
+#define TMP_IN 1
+#define TMP_REJ 2
+#define TMP_PAT 3
+
/* handy definitions */
#define strNE(s1,s2) (strcmp(s1, s2))
@@ -76,9 +81,7 @@ extern char *outname;
extern char *origprae;
extern char *TMPOUTNAME;
-extern char *TMPINNAME;
extern char *TMPREJNAME;
-extern char *TMPPATNAME;
extern bool toutkeep;
extern bool trejkeep;
Index: inp.c
===================================================================
RCS file: /cvs/src/usr.bin/patch/inp.c,v
retrieving revision 1.42
diff -u -p -u -p -r1.42 inp.c
--- inp.c 9 Dec 2014 20:28:43 -0000 1.42
+++ inp.c 13 Dec 2014 19:38:19 -0000
@@ -44,6 +44,7 @@
#include "util.h"
#include "pch.h"
#include "inp.h"
+#include "temp.h"
/* Input-file-with-indexable-lines abstract type */
@@ -52,6 +53,7 @@ static off_t i_size; /* size of the inp
static char *i_womp; /* plan a buffer for entire file */
static char **i_ptr; /* pointers to lines in i_womp */
+static char *TMPINNAME; /* plan b buffer file name */
static int tifd = -1; /* plan b virtual string array */
static char *tibuf[2]; /* plan b buffers */
static LINENUM tiline[2] = {-1, -1}; /* 1st line in each buffer */
@@ -341,9 +343,7 @@ plan_b(const char *filename)
using_plan_a = false;
if ((ifp = fopen(filename, "r")) == NULL)
pfatal("can't open file %s", filename);
- (void) unlink(TMPINNAME);
- if ((tifd = open(TMPINNAME, O_EXCL | O_CREAT | O_WRONLY, 0666)) < 0)
- pfatal("can't open file %s", TMPINNAME);
+ tifd = fileno(open_temp_file(TMP_IN, "r+", &TMPINNAME));
while ((p = fgetln(ifp, &len)) != NULL) {
if (p[len - 1] == '\n')
p[len - 1] = '\0';
@@ -419,9 +419,6 @@ plan_b(const char *filename)
p[j] = '\n';
}
fclose(ifp);
- close(tifd);
- if ((tifd = open(TMPINNAME, O_RDONLY)) < 0)
- pfatal("can't reopen file %s", TMPINNAME);
}
/*
Index: patch.c
===================================================================
RCS file: /cvs/src/usr.bin/patch/patch.c,v
retrieving revision 1.54
diff -u -p -u -p -r1.54 patch.c
--- patch.c 13 Dec 2014 10:31:07 -0000 1.54
+++ patch.c 13 Dec 2014 19:38:20 -0000
@@ -43,6 +43,7 @@
#include "inp.h"
#include "backupfile.h"
#include "pathnames.h"
+#include "temp.h"
mode_t filemode = 0644;
@@ -58,9 +59,7 @@ bool ok_to_create_file = false;
char *outname = NULL;
char *origprae = NULL;
char *TMPOUTNAME;
-char *TMPINNAME;
char *TMPREJNAME;
-char *TMPPATNAME;
bool toutkeep = false;
bool trejkeep = false;
bool warn_on_invalid_line;
@@ -91,8 +90,8 @@ static void abort_context_hunk(void);
static void rej_line(int, LINENUM);
static void abort_hunk(void);
static void apply_hunk(LINENUM);
-static void init_output(const char *);
-static void init_reject(const char *);
+static void init_output(void);
+static void init_reject(void);
static void copy_till(LINENUM, bool);
static void spew_output(void);
static void dump_line(LINENUM, bool);
@@ -141,10 +140,9 @@ static char end_defined[128];
int
main(int argc, char *argv[])
{
- int error = 0, hunk, failed, i, fd;
+ int error = 0, hunk, failed, i;
bool patch_seen;
LINENUM where = 0, newwhere, fuzz, mymaxfuzz;
- const char *tmpdir;
char *v;
setvbuf(stdout, NULL, _IOLBF, 0);
@@ -152,36 +150,6 @@ main(int argc, char *argv[])
for (i = 0; i < MAXFILEC; i++)
filearg[i] = NULL;
- /* Cons up the names of the temporary files. */
- if ((tmpdir = getenv("TMPDIR")) == NULL || *tmpdir == '\0')
- tmpdir = _PATH_TMP;
- for (i = strlen(tmpdir) - 1; i > 0 && tmpdir[i] == '/'; i--)
- ;
- i++;
- if (asprintf(&TMPOUTNAME, "%.*s/patchoXXXXXXXXXX", i, tmpdir) == -1)
- fatal("cannot allocate memory");
- if ((fd = mkstemp(TMPOUTNAME)) < 0)
- pfatal("can't create %s", TMPOUTNAME);
- close(fd);
-
- if (asprintf(&TMPINNAME, "%.*s/patchiXXXXXXXXXX", i, tmpdir) == -1)
- fatal("cannot allocate memory");
- if ((fd = mkstemp(TMPINNAME)) < 0)
- pfatal("can't create %s", TMPINNAME);
- close(fd);
-
- if (asprintf(&TMPREJNAME, "%.*s/patchrXXXXXXXXXX", i, tmpdir) == -1)
- fatal("cannot allocate memory");
- if ((fd = mkstemp(TMPREJNAME)) < 0)
- pfatal("can't create %s", TMPREJNAME);
- close(fd);
-
- if (asprintf(&TMPPATNAME, "%.*s/patchpXXXXXXXXXX", i, tmpdir) == -1)
- fatal("cannot allocate memory");
- if ((fd = mkstemp(TMPPATNAME)) < 0)
- pfatal("can't create %s", TMPPATNAME);
- close(fd);
-
v = getenv("SIMPLE_BACKUP_SUFFIX");
if (v)
simple_backup_suffix = v;
@@ -222,10 +190,10 @@ main(int argc, char *argv[])
}
/* initialize the patched file */
if (!skip_rest_of_patch)
- init_output(TMPOUTNAME);
+ init_output();
/* initialize reject file */
- init_reject(TMPREJNAME);
+ init_reject();
/* find out where all the lines are */
if (!skip_rest_of_patch)
@@ -910,22 +878,18 @@ apply_hunk(LINENUM where)
* Open the new file.
*/
static void
-init_output(const char *name)
+init_output(void)
{
- ofp = fopen(name, "w");
- if (ofp == NULL)
- pfatal("can't create %s", name);
+ ofp = open_temp_file(TMP_OUT, "w", &TMPOUTNAME);
}
/*
* Open a file to put hunks we can't locate.
*/
static void
-init_reject(const char *name)
+init_reject(void)
{
- rejfp = fopen(name, "w");
- if (rejfp == NULL)
- pfatal("can't create %s", name);
+ rejfp = open_temp_file(TMP_REJ, "w", &TMPREJNAME);
}
/*
Index: pch.c
===================================================================
RCS file: /cvs/src/usr.bin/patch/pch.c,v
retrieving revision 1.49
diff -u -p -u -p -r1.49 pch.c
--- pch.c 13 Dec 2014 10:31:07 -0000 1.49
+++ pch.c 13 Dec 2014 19:38:20 -0000
@@ -41,6 +41,7 @@
#include "util.h"
#include "pch.h"
#include "pathnames.h"
+#include "temp.h"
/* Patch (diff listing) abstract type. */
@@ -102,17 +103,15 @@ open_patch_file(const char *filename)
struct stat filestat;
if (filename == NULL || *filename == '\0' || strEQ(filename, "-")) {
- pfp = fopen(TMPPATNAME, "w");
- if (pfp == NULL)
- pfatal("can't create %s", TMPPATNAME);
+ pfp = open_temp_file(TMP_PAT, "r+", (char **)&filename);
while (fgets(buf, sizeof buf, stdin) != NULL)
fputs(buf, pfp);
- fclose(pfp);
- filename = TMPPATNAME;
+ fseeko(pfp, 0, SEEK_SET);
+ } else {
+ pfp = fopen(filename, "r");
+ if (pfp == NULL)
+ pfatal("patch file %s not found", filename);
}
- pfp = fopen(filename, "r");
- if (pfp == NULL)
- pfatal("patch file %s not found", filename);
if (fstat(fileno(pfp), &filestat))
pfatal("can't stat %s", filename);
p_filesize = filestat.st_size;
@@ -1377,6 +1376,8 @@ do_ed_script(void)
char *t;
off_t beginning_of_this_line;
FILE *pipefp = NULL;
+
+ fclose(open_temp_file(TMP_OUT, "w", &TMPOUTNAME));
if (!skip_rest_of_patch) {
if (copy_file(filearg[0], TMPOUTNAME) < 0) {
Index: temp.c
===================================================================
RCS file: temp.c
diff -N temp.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ temp.c 13 Dec 2014 19:38:20 -0000
@@ -0,0 +1,110 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2014 Tobias Stoeckmann <[email protected]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <paths.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "common.h"
+#include "util.h"
+#include "temp.h"
+
+static char *pattern[] = {
+ "patchoXXXXXXXXXX",
+ "patchiXXXXXXXXXX",
+ "patchrXXXXXXXXXX",
+ "patchpXXXXXXXXXX"
+};
+
+static char *path[4];
+static char *tmpdir;
+
+static void temp_init(void);
+
+static void
+temp_init(void)
+{
+ int i;
+
+ /* Cons up the names of the temporary files. */
+ if ((tmpdir = getenv("TMPDIR")) == NULL || *tmpdir == '\0')
+ tmpdir = _PATH_TMP;
+ for (i = strlen(tmpdir) - 1; i > 0 && tmpdir[i] == '/'; i--)
+ ;
+ i++;
+
+ if (asprintf(&tmpdir, "%.*s", i, tmpdir) == -1)
+ fatal("can't allocate memory");
+}
+
+FILE *
+open_temp_file(unsigned int type, const char *mode, char **name)
+{
+ FILE *fp;
+ int fd;
+
+ if (type > 3)
+ fatal("invalid temp type %u", type);
+
+ if (tmpdir == NULL)
+ temp_init();
+
+ close_temp_file(type);
+
+ if (asprintf(&path[type], "%s/%s", tmpdir, pattern[type]) == -1)
+ fatal("can't allocate memory");
+ if ((fd = mkstemp(path[type])) < 0) {
+ free(path[type]);
+ path[type] = NULL;
+ pfatal("can't create %s/%s", tmpdir, pattern[type]);
+ }
+
+ if ((fp = fdopen(fd, mode)) == NULL)
+ pfatal("can't fdopen %s", path[type]);
+ if (name != NULL)
+ *name = path[type];
+ return fp;
+}
+
+void
+close_temp_file(unsigned int type)
+{
+ if (type > 3)
+ fatal("invalid temp type %u", type);
+
+ if (path[type] != NULL) {
+ switch (type) {
+ case TMP_OUT:
+ if (!toutkeep)
+ unlink(path[type]);
+ toutkeep = false;
+ break;
+ case TMP_REJ:
+ if (!trejkeep)
+ unlink(path[type]);
+ trejkeep = false;
+ break;
+ default:
+ unlink(path[type]);
+ break;
+ }
+ free(path[type]);
+ path[type] = NULL;
+ }
+}
Index: temp.h
===================================================================
RCS file: temp.h
diff -N temp.h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ temp.h 13 Dec 2014 19:38:20 -0000
@@ -0,0 +1,19 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2014 Tobias Stoeckmann <[email protected]>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+FILE *open_temp_file(unsigned int, const char *, char **);
+void close_temp_file(unsigned int);
Index: util.c
===================================================================
RCS file: /cvs/src/usr.bin/patch/util.c,v
retrieving revision 1.38
diff -u -p -u -p -r1.38 util.c
--- util.c 13 Dec 2014 10:31:07 -0000 1.38
+++ util.c 13 Dec 2014 19:38:20 -0000
@@ -45,6 +45,7 @@
#include "util.h"
#include "backupfile.h"
#include "pathnames.h"
+#include "temp.h"
/* Rename a file, copying it if necessary. */
@@ -426,11 +427,9 @@ version(void)
void
my_exit(int status)
{
- unlink(TMPINNAME);
- if (!toutkeep)
- unlink(TMPOUTNAME);
- if (!trejkeep)
- unlink(TMPREJNAME);
- unlink(TMPPATNAME);
+ close_temp_file(TMP_IN);
+ close_temp_file(TMP_OUT);
+ close_temp_file(TMP_REJ);
+ close_temp_file(TMP_PAT);
exit(status);
}
