Bug 505257 (CVE-2009-1962)
Summary: | CVE-2009-1962 xfig: insecure use of temporary files | ||
---|---|---|---|
Product: | [Other] Security Response | Reporter: | Tomas Hoger <thoger> |
Component: | vulnerability | Assignee: | Red Hat Product Security <security-response-team> |
Status: | CLOSED WONTFIX | QA Contact: | |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | unspecified | CC: | bressers, hdegoede, kasal, pertusus, than |
Target Milestone: | --- | Keywords: | Security |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2009-1962 | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-08-21 22:39:11 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Tomas Hoger
2009-06-11 08:43:26 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 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. |