[2963] in bugtraq
Re: BSD mail.local has race condition - fix
daemon@ATHENA.MIT.EDU (Travis Hassloch x231)
Thu Jul 18 18:28:02 1996
Date: Thu, 18 Jul 1996 16:42:43 -0500
Reply-To: Bugtraq List <BUGTRAQ@netspace.org>
From: Travis Hassloch x231 <travis@EvTech.com>
To: Multiple recipients of list BUGTRAQ <BUGTRAQ@netspace.org>
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.
