Bug 505257 (CVE-2009-1962) - CVE-2009-1962 xfig: insecure use of temporary files
Summary: CVE-2009-1962 xfig: insecure use of temporary files
Keywords:
Status: CLOSED WONTFIX
Alias: CVE-2009-1962
Product: Security Response
Classification: Other
Component: vulnerability
Version: unspecified
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Red Hat Product Security
QA Contact:
URL: http://web.nvd.nist.gov/view/vuln/det...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-06-11 08:43 UTC by Tomas Hoger
Modified: 2019-09-29 12:30 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-08-21 22:39:11 UTC
Embargoed:


Attachments (Terms of Use)

Description Tomas Hoger 2009-06-11 08:43:26 UTC
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. 

References:
http://www.openwall.com/lists/oss-security/2009/04/01/6
http://www.securityfocus.com/bid/34328
http://xforce.iss.net/xforce/xfdb/49600

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

Comment 2 Tomas Hoger 2009-06-11 09:46:16 UTC
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.


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