Bug 505257 (CVE-2009-1962)

In Red Hat Enterprise Linux (3+) and Fedora XFig packages, we have most of these cases covered thanks to xfig-3.2.X-mkstemp.diff patches, which seem to have been added some time in RHL 6.x timeframe (bug #67351).

The patch against Debian packages that was posted with the report seems to be a copy of the xfig-3.2.5-mkstemp.diff from Fedora.  However, compared to 3.2.3 and 3.2.4 patches in RHEL, it is missing one hunk that got lost during 3.2.4 -> 3.2.5 forward-port for this case:

u_print.c:    sprintf(tmp_fig_file, "%s/%s%06d", TMPDIR, "xfig-fig", getpid());

Further details about this case and other remaining suspicious TMPDIR uses in my post here:

http://thread.gmane.org/gmane.comp.security.oss.general/1844/focus=1848

Regarding remaining cases:

d_text.c:  sprintf(preedit_filename, "%s/%s%06d", TMPDIR, "xfig-preedit", getpid());

As noted in the referenced post, this should not be used in our builds.  Please correct me if I'm wrong.  Anyway, this should be easy to fix using mkstemp in a similar way other cases are fixed.


Other two cases are not really temporary files, so randomized names may not be usable there.  We can instead make sure we can open O_EXCL file with intended name, or fail.  So probably something like this can be done (I've not tested the fixes, they may even fail to compile ;).

f_util.c:     sprintf(tmpfile, "%s%s", TMPDIR, c);
f_util.c:     sprintf(tmpfile, "%s/%s", TMPDIR, plainname);

--- f_util.c.orig	2009-06-11 11:22:32.000000000 +0200
+++ f_util.c	2009-06-11 11:25:04.000000000 +0200
@@ -742,6 +742,7 @@ uncompress_file(char *name)
     char	    unc[PATH_MAX+20];	/* temp buffer for uncompress/gunzip command */
     char	   *c;
     struct stat	    status;
+    int            fd;
 
     strcpy(tmpfile, name);		/* save original name */
     strcpy(plainname, name);
@@ -792,7 +793,12 @@ uncompress_file(char *name)
 	  sprintf(tmpfile, "%s%s", TMPDIR, c);
       else
 	  sprintf(tmpfile, "%s/%s", TMPDIR, plainname);
-      sprintf(unc, "gunzip -q -c %s > %s", name, tmpfile);
+      if ((fd = open(tmpfile, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL, 0666)) == -1)
+        sprintf(unc, "/bin/false");
+      else {
+        close(fd);
+        sprintf(unc, "gunzip -q -c %s > %s", name, tmpfile);
+      }
       if (system(unc) != 0)
 	  file_msg("Couldn't uncompress the file: \"%s\"", unc);
       file_msg ("Uncompressing file %s in %s because it is in a read-only directory",


u_error.c:      if (emergency_save(strcat(TMPDIR,"/SAVE.fig")) == -1)

--- f_save.c.orig	2009-06-11 11:06:10.000000000 +0200
+++ f_save.c	2009-06-11 11:16:38.000000000 +0200
@@ -589,8 +589,14 @@ void write_comments(FILE *fp, char *com)
 int emergency_save(char *file_name)
 {
     FILE	   *fp;
+    int 	   fd
+    int 	   flags = O_WRONLY|O_CREAT|O_TRUNC;
 
-    if ((fp = fopen(file_name, "wb")) == NULL)
+    if (strcmp(file_name, TMPDIR) == 0)
+	flags |= O_EXCL;
+    if ((fd = open(file_name, flags, 0666)) == -1)
+	return (-1);
+    if ((fp = fdopen(fd, "wb")) == NULL)
 	return (-1);
     /* first close any open compounds */
     close_all_compounds();


We may wish to more restrictive mode by default possibly and not rely on user's umask.