Bug 505257 - (CVE-2009-1962) CVE-2009-1962 xfig: insecure use of temporary files
CVE-2009-1962 xfig: insecure use of temporary files
Product: Security Response
Classification: Other
Component: vulnerability (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Red Hat Product Security
: Security
Depends On:
  Show dependency treegraph
Reported: 2009-06-11 04:43 EDT by Tomas Hoger
Modified: 2015-08-21 18:39 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2015-08-21 18:39:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Tomas Hoger 2009-06-11 04:43:26 EDT
Common Vulnerabilities and Exposures assigned an identifier CVE-2009-1962 to the following vulnerability:

Xfig in Debian GNU/Linux, possibly 3.2.5, allows local users to read
and write arbitrary files via a symlink attack on the (1)
xfig-eps[PID], (2) xfig-pic[PID].pix, (3) xfig-pic[PID].err, (4)
xfig-pcx[PID].pix, (5) xfig-xfigrc[PID], (6) xfig[PID], (7)
xfig-print[PID], (8) xfig-export[PID].err, (9) xfig-batch[PID], (10)
xfig-exp[PID], or (11) xfig-spell.[PID] temporary files, where [PID]
is a process ID. 

Comment 1 Tomas Hoger 2009-06-11 04:50:47 EDT
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:

Comment 2 Tomas Hoger 2009-06-11 05:46:16 EDT
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);
 	  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 */

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

Note You need to log in before you can comment on or make changes to this bug.