Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Jul 1996 16:42:43 -0500
From:      Travis Hassloch x231 <travis@EvTech.com>
To:        bugtraq@netspace.org
Cc:        freebsd-current@freebsd.org, current-users@NetBSD.ORG, cross@math.psu.edu
Subject:   Re: BSD mail.local has race condition - fix
Message-ID:  <199607182142.QAA19123@tahiti.evtech.com>

next in thread | raw e-mail | index | archive | help
This fix is relative to:
static char sccsid[] = "from: @(#)mail.local.c        5.6 (Berkeley) 6/19/91";
as modified into:
  "$Id: mail.local.c,v 1.1.1.1 1995/10/18 08:43:19 deraadt Exp $"

The problem I described does _not_ occur in BSD/OS.

I have cc'ed everyone I think might want to integrate it ('cept OpenBSD,
Theo already has a copy); the rest is up to you.  I believe it to fix
the problem but have not even compiled it as I lack a BSD environment here.

--- mail.local.c~	Tue Jul 16 19:00:07 1996
+++ mail.local.c	Tue Jul 16 21:02:15 1996
@@ -168,9 +168,9 @@
 	char *name;
 	int lockfile;
 {
-	struct stat sb;
+	struct stat sb, fsb;
 	struct passwd *pw;
-	int created, mbfd, nr, nw, off, rval=0, lfd=-1;
+	int mbfd=-1, nr, nw, off, rval=1, lfd=-1;
 	char biffmsg[100], buf[8*1024], path[MAXPATHLEN], lpath[MAXPATHLEN];
 	off_t curoff;
 
@@ -194,59 +194,94 @@
 			return(1);
 		}
 	}
-
-	if (!(created = lstat(path, &sb)) &&
-	    (sb.st_nlink != 1 || S_ISLNK(sb.st_mode))) {
-		err(NOTFATAL, "%s: linked file", path);
-		return(1);
-	}
-	if((mbfd = open(path, O_APPEND|O_WRONLY|O_EXLOCK,
-	    S_IRUSR|S_IWUSR)) < 0) {
+	/* after this point, always exit via bad to remove lockfile */
+retry:
+	if (lstat(path, &sb)) {
+		if (errno != ENOENT) {
+			err(NOTFATAL, "%s: %s", path, strerror(errno));
+			goto bad;
+		}
 		if ((mbfd = open(path, O_APPEND|O_CREAT|O_WRONLY|O_EXLOCK,
-		    S_IRUSR|S_IWUSR)) < 0) {
-		err(NOTFATAL, "%s: %s", path, strerror(errno));
-		return(1);
+		     S_IRUSR|S_IWUSR)) < 0) {
+			if (errno == EEXIST) {
+				/* file appeared since lstat */
+				goto retry;
+			}
+			else {
+				err(NOTFATAL, "%s: %s", path, strerror(errno));
+				goto bad;
+			}
+		}
+	/*
+	 * Set the owner and group.  Historically, binmail repeated this at
+	 * each mail delivery.  We no longer do this, assuming that if the
+	 * ownership or permissions were changed there was a reason for doing
+	 * so.
+	 */
+		if (fchown(mbfd, pw->pw_uid, pw->pw_gid) < 0) {
+			err(NOTFATAL, "chown %u:%u: %s",
+			    pw->pw_uid, pw->pw_gid, name);
+			goto bad;
+		}
 	}
+	else {
+		if (sb.st_nlink != 1 || S_ISLNK(sb.st_mode)) {
+			err(NOTFATAL, "%s: linked file", path);
+			goto bad;
+		}
+		if ((mbfd = open(path, O_APPEND|O_WRONLY|O_EXLOCK,
+		    S_IRUSR|S_IWUSR)) < 0) {
+			err(NOTFATAL, "%s: %s", path, strerror(errno));
+			goto bad;
+		}
+		if (fstat(mbfd, &fsb)) {
+			/* relating error to path may be bad style */
+			err(NOTFATAL, "%s: %s", path, strerror(errno));
+			goto bad;
+		}
+		if (sb.st_dev != fsb.st_dev || sb.st_ino != fsb.st_ino) {
+			err(NOTFATAL, "%s: changed after open", path);
+			goto bad;
+		}
+		/* paranoia? */
+		if (fsb.st_nlink != 1 || S_ISLNK(fsb.st_mode)) {
+			err(NOTFATAL, "%s: linked file", path);
+			goto bad;
+		}
 	}
 
 	curoff = lseek(mbfd, 0, SEEK_END);
 	(void)sprintf(biffmsg, "%s@%qd\n", name, curoff);
 	if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
 		err(FATAL, "temporary file: %s", strerror(errno));
-		rval = 1;
 		goto bad;
 	}
 
+	/* This section (to trunc) is ugly */
 	while ((nr = read(fd, buf, sizeof(buf))) > 0)
 		for (off = 0; off < nr;  off += nw)
 			if ((nw = write(mbfd, buf + off, nr - off)) < 0) {
 				err(NOTFATAL, "%s: %s", path, strerror(errno));
 				goto trunc;
 			}
-	if (nr < 0) {
+	if (!nr) {
+		rval = 0;
+	}
+	else {
 		err(FATAL, "temporary file: %s", strerror(errno));
 trunc:		(void)ftruncate(mbfd, curoff);
-		rval = 1;
 	}
 
-	/*
-	 * Set the owner and group.  Historically, binmail repeated this at
-	 * each mail delivery.  We no longer do this, assuming that if the
-	 * ownership or permissions were changed there was a reason for doing
-	 * so.
-	 */
 bad:
-	if(lockfile) {
-		if(lfd >= 0) {
-			unlink(lpath);
-			close(lfd);
-		}
+	if(lfd >= 0) {
+		unlink(lpath);
+		close(lfd);
 	}
-	if (created) 
-		(void)fchown(mbfd, pw->pw_uid, pw->pw_gid);
 
-	(void)fsync(mbfd);		/* Don't wait for update. */
-	(void)close(mbfd);		/* Implicit unlock. */
+	if (mbfd >= 0) {
+		(void)fsync(mbfd);		/* Don't wait for update. */
+		(void)close(mbfd);		/* Implicit unlock. */
+	}
 
 	if (!rval)
 		notifybiff(biffmsg);
--
Travis Hassloch, Electronic Blacksmith | P=NP if (P=0 or N=1)
There is a fine line between an email message and it's signature.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199607182142.QAA19123>