Date: Wed, 27 May 1998 01:02:59 -0400 (EDT) From: Allen Smith <root@PCK-4.rutgers.edu> To: FreeBSD-gnats-submit@FreeBSD.ORG Subject: misc/6773: Improving security of tempnam.c Message-ID: <199805270502.BAA15321@PCK-4.rutgers.edu>
next in thread | raw e-mail | index | archive | help
>Number: 6773
>Category: misc
>Synopsis: tempnam.c security problems
>Confidential: no
>Severity: serious
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: change-request
>Submitter-Id: current-users
>Arrival-Date: Wed May 27 14:00:03 PDT 1998
>Last-Modified:
>Originator: Allen Smith
>Organization:
Rutgers University
>Release: FreeBSD 2.2.6-STABLE i386
>Environment:
None
>Description:
The tempnam function has the security problem of trusting an
environment variable, even when running setuid. While it might seem that using
this function in a setuid/setgid program is insecure in and of itself due to
the potential race condition between finding a file name and creating the
file, open with the CREAT _and_ EXCL flags set solves this problem, at least
for local filesystems. (Yes, the mkstemp function should be used instead, but
software ported in from other OSes may not use this function.)
The difficulty with trusting a user-set directory when this method
(setting CREAT and EXCL) is used is that the directory in question could have
permissions allowing the user to replace the file - a problem if the program
closes the file then reopens it at a later point (perhaps to conserve file
descriptors). It also poses the confidentiality hazard that the directory may
be set to be in a msdos filesystem, so that the user can then examine the
contents of the file.
>How-To-Repeat:
See description; it should be obvious.
>Fix:
Apply this patch to tempnam.c (in /usr/src/lib/libc/stdio); if
somebody would also patch the manpage, that would be nice:
--- tempnam.c.old Tue May 26 06:12:24 1998
+++ tempnam.c Wed May 27 00:50:26 1998
@@ -52,7 +52,8 @@
const char *dir, *pfx;
{
int sverrno;
- char *f, *name;
+ char *tmpdir, *f, *name;
+ int safe = 0; /* safe to trust TMPDIR environment variable - Allen */
if (!(name = malloc(MAXPATHLEN)))
return(NULL);
@@ -60,12 +61,25 @@
if (!pfx)
pfx = "tmp.";
- if ((f = getenv("TMPDIR"))) {
- (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", f,
- *(f + strlen(f) - 1) == '/'? "": "/", pfx);
- if ((f = mktemp(name)))
- return(f);
- }
+ if ((tmpdir = getenv("TMPDIR")))
+ {
+ if (issetugid() && getuid())
+ {
+ safe = 0;
+ }
+ else
+ {
+ safe = 1;
+ }
+ }
+
+ if (safe)
+ {
+ (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", tmpdir,
+ *(tmpdir + strlen(tmpdir) - 1) == '/'? "": "/", pfx);
+ if ((f = mktemp(name)))
+ return(f);
+ }
if ((f = (char *)dir)) {
(void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", f,
@@ -83,6 +97,19 @@
(void)snprintf(name, MAXPATHLEN, "%s%sXXXXXX", f, pfx);
if ((f = mktemp(name)))
return(f);
+
+ f = _PATH_VARTMP; /* to try to not use the unsafe fallback option - Allen */
+ (void)snprintf(name, MAXPATHLEN, "%s%sXXXXXX", f, pfx);
+ if ((f= mktemp(name)))
+ return(f);
+
+ if ((!safe) && tmpdir) /* to insure compatibility, trust it as a fallback option - Allen */
+ {
+ (void)snprintf(name, MAXPATHLEN, "%s%s%sXXXXXX", tmpdir,
+ *(tmpdir + strlen(tmpdir) - 1) == '/'? "": "/", pfx);
+ if ((f = mktemp(name)))
+ return(f);
+ }
sverrno = errno;
free(name);
>Audit-Trail:
>Unformatted:
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199805270502.BAA15321>