From owner-freebsd-bugs  Wed May 27 14:05:42 1998
Return-Path: <owner-freebsd-bugs@FreeBSD.ORG>
Received: (from majordom@localhost)
          by hub.freebsd.org (8.8.8/8.8.8) id OAA00212
          for freebsd-bugs-outgoing; Wed, 27 May 1998 14:05:42 -0700 (PDT)
          (envelope-from owner-freebsd-bugs@FreeBSD.ORG)
Received: from freefall.freebsd.org (freefall.FreeBSD.ORG [204.216.27.21])
          by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id OAA00202
          for <freebsd-bugs@FreeBSD.org>; Wed, 27 May 1998 14:05:37 -0700 (PDT)
          (envelope-from gnats@FreeBSD.org)
Received: (from gnats@localhost)
	by freefall.freebsd.org (8.8.8/8.8.5) id OAA18265;
	Wed, 27 May 1998 14:00:04 -0700 (PDT)
Received: from PCK-4.rutgers.edu (pck-4.rutgers.edu [165.230.209.132])
          by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id NAA28397
          for <FreeBSD-gnats-submit@freebsd.org>; Wed, 27 May 1998 13:58:56 -0700 (PDT)
          (envelope-from root@PCK-4.rutgers.edu)
Received: (from root@localhost)
	by PCK-4.rutgers.edu (8.8.8/8.8.8) id BAA15321;
	Wed, 27 May 1998 01:02:59 -0400 (EDT)
	(envelope-from root)
Message-Id: <199805270502.BAA15321@PCK-4.rutgers.edu>
Date: Wed, 27 May 1998 01:02:59 -0400 (EDT)
From: Allen Smith <root@PCK-4.rutgers.edu>
Reply-To: easmith@beatrice.rutgers.edu
To: FreeBSD-gnats-submit@FreeBSD.ORG
X-Send-Pr-Version: 3.2
Subject: misc/6773: Improving security of tempnam.c
Sender: owner-freebsd-bugs@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org


>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