Hi,
The people at Immunix recently scanned all of RedHat 7.0 for
temp file problems and found some in CVS (among many others).
I'm currently testing a patch for this problem; the current
patch is attached.
What the patch does is
- define CVS_SAFE_FOPEN and safe_fopen to create
temp files safely (i.e. using O_EXCL). This is still
subject to denial of service, but at least it's safe :)
- Checked all calls to cvs_temp_name(), and made sure
that the resulting file is opened using safe_fopen()
In most cases this was straightforward, but on several
occasions RCS_checkout is called, and I went through
RCS_checkout to make sure the file is created safely
(this part of the patch may need special attention
to make sure it's okay)
Cheers
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[EMAIL PROTECTED] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
[EMAIL PROTECTED] +-------------------- Why Not?! -----------------------
UNIX, n.: Spanish manufacturer of fire extinguishers.
diff -ur --new-file cvs-1.11.orig/lib/Makefile.in cvs-1.11/lib/Makefile.in
--- cvs-1.11.orig/lib/Makefile.in Thu Dec 30 03:44:54 1999
+++ cvs-1.11/lib/Makefile.in Fri Jan 5 11:06:21 2001
@@ -39,6 +39,7 @@
mkdir.c \
regex.c \
rename.c \
+ safe_fopen.c \
savecwd.c \
sighandle.c \
strstr.c \
@@ -50,7 +51,8 @@
xgetwd.c \
yesno.c
-HEADERS = getline.h getopt.h fnmatch.h regex.h system.h wait.h md5.h savecwd.h
+HEADERS = getline.h getopt.h fnmatch.h regex.h system.h wait.h md5.h savecwd.h \
+ safe_fopen.h
# Always use CVS's regular expression matcher regex.o, because of
# variations in regular expression syntax - we want to be the same
@@ -70,6 +72,7 @@
getopt1.o \
md5.o \
regex.o \
+ safe_fopen.o \
savecwd.o \
sighandle.o \
stripslash.o \
diff -ur --new-file cvs-1.11.orig/lib/safe_fopen.c cvs-1.11/lib/safe_fopen.c
--- cvs-1.11.orig/lib/safe_fopen.c Thu Jan 1 01:00:00 1970
+++ cvs-1.11/lib/safe_fopen.c Fri Jan 5 11:06:36 2001
@@ -0,0 +1,24 @@
+/*
+ * Safely open a temp file
+ *
+ */
+
+#include <stdio.h>
+#include "system.h"
+
+FILE *
+safe_fopen(const char *name, const char *mode)
+{
+ int fd;
+
+ /* Accept modes w w+ wb etc */
+ if (mode[0] != 'w' && mode[0] != 'a') {
+ errno = EINVAL;
+ return NULL;
+ }
+
+ if ((fd = open(name, O_RDWR|O_CREAT|O_EXCL, 0600)) < 0)
+ return NULL;
+
+ return fdopen(fd, mode);
+}
diff -ur --new-file cvs-1.11.orig/lib/safe_fopen.h cvs-1.11/lib/safe_fopen.h
--- cvs-1.11.orig/lib/safe_fopen.h Thu Jan 1 01:00:00 1970
+++ cvs-1.11/lib/safe_fopen.h Fri Jan 5 11:06:36 2001
@@ -0,0 +1,14 @@
+#ifndef SAFE_FOPEN_H
+#define SAFE_FOPEN_H
+
+#include <stdio.h>
+
+#if defined (__GNUC__) || (defined (__STDC__) && __STDC__)
+#define __PROTO(args) args
+#else
+#define __PROTO(args) ()
+#endif /* GCC. */
+
+extern FILE * safe_fopen __PROTO((const char *, const char *));
+
+#endif /* SAFE_FOPEN_H */
Binary files cvs-1.11.orig/lib/safe_fopen.o and cvs-1.11/lib/safe_fopen.o differ
diff -ur --new-file cvs-1.11.orig/lib/system.h cvs-1.11/lib/system.h
--- cvs-1.11.orig/lib/system.h Mon Feb 16 19:55:18 1998
+++ cvs-1.11/lib/system.h Fri Jan 5 11:06:21 2001
@@ -429,6 +429,11 @@
#define CVS_FOPEN fopen
#endif
+#include "safe_fopen.h"
+#ifndef CVS_SAFE_FOPEN
+#define CVS_SAFE_FOPEN safe_fopen
+#endif
+
#ifndef CVS_MKDIR
#define CVS_MKDIR mkdir
#endif
diff -ur --new-file cvs-1.11.orig/src/commit.c cvs-1.11/src/commit.c
--- cvs-1.11.orig/src/commit.c Wed Jul 26 21:29:01 2000
+++ cvs-1.11/src/commit.c Fri Jan 5 11:06:21 2001
@@ -592,7 +592,7 @@
FILE *fp;
fname = cvs_temp_name ();
- fp = CVS_FOPEN (fname, "w+");
+ fp = CVS_SAFE_FOPEN (fname, "w+");
if (fp == NULL)
error (1, 0, "cannot create temporary file %s", fname);
if (fwrite (saved_message, 1, strlen (saved_message), fp)
diff -ur --new-file cvs-1.11.orig/src/import.c cvs-1.11/src/import.c
--- cvs-1.11.orig/src/import.c Tue Jul 11 22:32:02 2000
+++ cvs-1.11/src/import.c Fri Jan 5 11:06:21 2001
@@ -291,7 +291,7 @@
/* Create the logfile that will be logged upon completion */
tmpfile = cvs_temp_name ();
- if ((logfp = CVS_FOPEN (tmpfile, "w+")) == NULL)
+ if ((logfp = CVS_SAFE_FOPEN (tmpfile, "w+")) == NULL)
error (1, errno, "cannot create temporary file `%s'", tmpfile);
/* On systems where we can unlink an open file, do so, so it will go
away no matter how we exit. FIXME-maybe: Should be checking for
diff -ur --new-file cvs-1.11.orig/src/login.c cvs-1.11/src/login.c
--- cvs-1.11.orig/src/login.c Tue Aug 1 18:17:50 2000
+++ cvs-1.11/src/login.c Fri Jan 5 11:06:21 2001
@@ -215,7 +215,7 @@
FILE *tmp_fp;
tmp_name = cvs_temp_name ();
- if ((tmp_fp = CVS_FOPEN (tmp_name, "w")) == NULL)
+ if ((tmp_fp = CVS_SAFE_FOPEN (tmp_name, "w")) == NULL)
{
error (1, errno, "unable to open temp file %s", tmp_name);
return 1;
@@ -450,7 +450,7 @@
/* FIXME: This should not be in /tmp; that is almost surely a security
hole. Probably should just keep it in memory. */
tmp_name = cvs_temp_name ();
- if ((tmp_fp = CVS_FOPEN (tmp_name, "w")) == NULL)
+ if ((tmp_fp = CVS_SAFE_FOPEN (tmp_name, "w")) == NULL)
{
error (1, errno, "unable to open temp file %s", tmp_name);
return 1;
diff -ur --new-file cvs-1.11.orig/src/logmsg.c cvs-1.11/src/logmsg.c
--- cvs-1.11.orig/src/logmsg.c Wed Mar 1 17:33:39 2000
+++ cvs-1.11/src/logmsg.c Fri Jan 5 11:06:21 2001
@@ -194,7 +194,11 @@
/* Create a temporary file */
fname = cvs_temp_name ();
again:
- if ((fp = CVS_FOPEN (fname, "w+")) == NULL)
+ /* need to remove file in case we jumped here after getting
+ * an error */
+ unlink(fname);
+
+ if ((fp = CVS_SAFE_FOPEN (fname, "w+")) == NULL)
error (1, 0, "cannot create temporary file %s", fname);
if (*messagep)
@@ -419,7 +423,7 @@
fname = cvs_temp_name ();
- fp = fopen (fname, "w");
+ fp = CVS_SAFE_FOPEN (fname, "w");
if (fp == NULL)
error (1, errno, "cannot create temporary file %s", fname);
else
diff -ur --new-file cvs-1.11.orig/src/patch.c cvs-1.11/src/patch.c
--- cvs-1.11.orig/src/patch.c Thu Jul 6 19:30:42 2000
+++ cvs-1.11/src/patch.c Fri Jan 5 11:06:21 2001
@@ -504,7 +504,7 @@
/* Create 3 empty files. I'm not really sure there is any advantage
to doing so now rather than just waiting until later. */
tmpfile1 = cvs_temp_name ();
- fp1 = CVS_FOPEN (tmpfile1, "w+");
+ fp1 = CVS_SAFE_FOPEN (tmpfile1, "w+");
if (fp1 == NULL)
{
error (0, errno, "cannot create temporary file %s", tmpfile1);
@@ -515,7 +515,7 @@
if (fclose (fp1) < 0)
error (0, errno, "warning: cannot close %s", tmpfile1);
tmpfile2 = cvs_temp_name ();
- fp2 = CVS_FOPEN (tmpfile2, "w+");
+ fp2 = CVS_SAFE_FOPEN (tmpfile2, "w+");
if (fp2 == NULL)
{
error (0, errno, "cannot create temporary file %s", tmpfile2);
@@ -526,7 +526,7 @@
if (fclose (fp2) < 0)
error (0, errno, "warning: cannot close %s", tmpfile2);
tmpfile3 = cvs_temp_name ();
- fp3 = CVS_FOPEN (tmpfile3, "w+");
+ fp3 = CVS_SAFE_FOPEN (tmpfile3, "w+");
if (fp3 == NULL)
{
error (0, errno, "cannot create temporary file %s", tmpfile3);
diff -ur --new-file cvs-1.11.orig/src/rcs.c cvs-1.11/src/rcs.c
--- cvs-1.11.orig/src/rcs.c Mon Aug 21 23:16:38 2000
+++ cvs-1.11/src/rcs.c Fri Jan 5 11:06:21 2001
@@ -4338,12 +4338,11 @@
ofp = stdout;
else
{
- /* Symbolic links should be removed before replacement, so that
- `fopen' doesn't follow the link and open the wrong file. */
- if (islink (sout))
- if (unlink_file (sout) < 0)
- error (1, errno, "cannot remove %s", sout);
- ofp = CVS_FOPEN (sout, expand == KFLAG_B ? "wb" : "w");
+ /* Unconditionally remove the old file, so that safe_fopen
+ * can succeed */
+ if (unlink_file (sout) < 0)
+ error (1, errno, "cannot remove %s", sout);
+ ofp = CVS_SAFE_FOPEN (sout, expand == KFLAG_B ? "wb" : "w");
if (ofp == NULL)
error (1, errno, "cannot open %s", sout);
}
diff -ur --new-file cvs-1.11.orig/src/rcscmds.c cvs-1.11/src/rcscmds.c
--- cvs-1.11.orig/src/rcscmds.c Sat Aug 15 22:47:46 1998
+++ cvs-1.11/src/rcscmds.c Fri Jan 5 11:06:21 2001
@@ -223,9 +223,17 @@
if (out == RUN_TTY)
return diff3_run (call_diff_argc, call_diff_argv, NULL,
&call_diff_stdout_callbacks);
- else
+ else {
+ FILE *fp;
+
+ /* Make sure the file is created safely */
+ if (!(fp = CVS_SAFE_FOPEN(out, "w")))
+ return 2;
+ fclose(fp);
+
return diff3_run (call_diff_argc, call_diff_argv, out,
&call_diff_file_callbacks);
+ }
}
diff -ur --new-file cvs-1.11.orig/src/wrapper.c cvs-1.11/src/wrapper.c
--- cvs-1.11.orig/src/wrapper.c Wed Dec 23 16:15:01 1998
+++ cvs-1.11/src/wrapper.c Fri Jan 5 11:06:21 2001
@@ -566,6 +566,15 @@
free (buf);
buf = cvs_temp_name ();
+ /* Create temp file safely */
+ {
+ FILE *fp;
+
+ if (!(fp = CVS_SAFE_FOPEN(buf, "w")))
+ return NULL;
+ fclose(fp);
+ }
+
args = xmalloc (strlen (e->tocvsFilter)
+ strlen (fileName)
+ strlen (buf));
