patch-2.5.4 mktemp fix

Radosław Krahl ptasz3k at o2.pl
Thu Jan 15 15:03:30 PST 2004


I have done a little patch which fixes the mktemp vulnerability in
patch-2.5.4a, so it now uses mkstemp instead now. I don't know if this is so
important, but if we patch ed against this vulnerability, then why we leave
it in patch, which is used much often. Anyway, here it is. It isn't too
clean, but it works. If you find it useful, use it :).
And sorry for my terrible english.

ptasz3k

patch is here (cut and paste text between ##PATCH_START and ##PATCH_END):

##PATCH_START
--- patch-2.5.4/patch.c 1999-08-30 08:20:08.000000000 +0200
+++ patch-2.5.4-fix-mktemp/patch.c 2004-01-15 07:12:00.000000000 +0100
@@ -71,7 +71,6 @@
 static void init_output PARAMS ((char const *, int, struct outstate *));
 static void init_reject PARAMS ((void));
 static void reinitialize_almost_everything PARAMS ((void));
-static void remove_if_needed PARAMS ((char const *, int volatile *));
 static void usage PARAMS ((FILE *, int)) __attribute__((noreturn));

 static int make_backups;
@@ -1154,11 +1153,9 @@
 static FILE *
 create_output_file (char const *name, int open_flags)
 {
-  int fd = create_file (name, O_WRONLY | binary_transput | open_flags,
-   instat.st_mode);
-  FILE *f = fdopen (fd, binary_transput ? "wb" : "w");
+  FILE *f = fopen (name, binary_transput ? "wb" : "w");
   if (! f)
-    pfatal ("Can't create file %s", quotearg (name));
+    pfatal ("Can't open file %s (mkstemp error)", quotearg (name));
   return f;
 }

@@ -1315,21 +1312,27 @@
 make_temp (int letter)
 {
   char *r;
-#if HAVE_MKTEMP
+  int   tfd; /* temporary file descriptor */
+  mode_t old_umask;
   char const *tmpdir = getenv ("TMPDIR"); /* Unix tradition */
   if (!tmpdir) tmpdir = getenv ("TMP");  /* DOS tradition */
   if (!tmpdir) tmpdir = getenv ("TEMP"); /* another DOS tradition */
   if (!tmpdir) tmpdir = TMPDIR;
   r = xmalloc (strlen (tmpdir) + 10);
   sprintf (r, "%s/p%cXXXXXX", tmpdir, letter);
-  mktemp (r);
-  if (!*r)
-    pfatal ("mktemp");
-#else
-  r = xmalloc (L_tmpnam);
-  if (! (tmpnam (r) == r && *r))
-    pfatal ("tmpnam");
-#endif
+  /* man page for mkstemp says:
+     "The  old  behaviour  (creating a file with mode 0666) may be a
security
+      risk, especially since other Unix flavours use 0600, and somebody
might
+      overlook this detail when porting programs.
+      More  generally,  the  POSIX  specification does not say anything
about
+      file modes, so the application should make sure its umask is set
appro-
+      priately before calling mkstemp."
+     So we save old umask and set mode 0600 before calling mkstemp. */
+  old_umask = umask (0177);
+  tfd = mkstemp (r);
+  umask (old_umask);
+  if (tfd == -1)
+    pfatal ("mkstemp");
   return r;
 }

@@ -1347,20 +1350,12 @@
 }

 static void
-remove_if_needed (char const *name, int volatile *needs_removal)
-{
-  if (*needs_removal)
-    {
-      unlink (name);
-      *needs_removal = 0;
-    }
-}
-
-static void
 cleanup (void)
 {
-  remove_if_needed (TMPINNAME, &TMPINNAME_needs_removal);
-  remove_if_needed (TMPOUTNAME, &TMPOUTNAME_needs_removal);
-  remove_if_needed (TMPPATNAME, &TMPPATNAME_needs_removal);
-  remove_if_needed (TMPREJNAME, &TMPREJNAME_needs_removal);
+  /* remove files created by mkstemp. we don't want to end up with ziliards
+     temporary patch files in /tmp */
+  unlink (TMPINNAME);
+  unlink (TMPOUTNAME);
+  unlink (TMPPATNAME);
+  unlink (TMPREJNAME);
 }
##PATCH_END




More information about the hlfs-dev mailing list