[comp.mail.elm] Dangerous "filter" bug found, and the fix

taylor@hplabs.HP.COM (Dave Taylor) (01/16/88)

While working on getting the latest version of Elm up and happy, I
noticed that the 'filter' program wasn't interacting with Elm too
well about lock files.  Further investigation showed that in fact
the locking routines used within the filter program are garbage
and, as far as I could tell, accomplish nothing.  Not good.

So I took the specific lock() and unlock() calls and put them in
a different file, rewriting "actions.c" along the way.  If you
are using "filter" then you should update to the version included
herein ASAP to ensure you don't have any problems (I lost about
ten messages out of my mailbox before I tracked it down).

And again, if anyone has noticed any other quirks in any of the parts 
of Elm, please drop me a note!

						-- Dave Taylor

-- -- -- partial shell archive for "filter" program: -- -- --

# Shell Archive created by taylor@atom.HPL.HP.COM at Fri Jan 15 11:11:49 1988

# To unpack the enclosed files, please use this file as input to the
# Bourne (sh) shell.  This can be most easily done by the command;
#     sh < thisfilename

# This archive contains;
#  Patch.Instruct   actions.c           lock.c


# ---------- file Patch.Instruct ----------

filename="Patch.Instruct"

if [ -f $filename ]
then
  echo File \"$filename\" already exists\!  Skipping...
  filename=/dev/null		# throw it away
else
  echo extracting file Patch.Instruct...
fi

cat << 'END-OF-FILE' > $filename


This patch to filter should be real easy to install: just
unpack the two files contained in the shar file (they are
"lock.c" and "actions.c") letting the latter replace the
existing one in your source tree, then go into your Makefile
("Elm/filter/Makefile") and change the SRC and OBJ targets
to include the new file "lock.c" as appropriate.

Then just type "make" for it to be rebuilt.

Sorry for the confusion.

Dave Taylor
END-OF-FILE

if [ "$filename" != "/dev/null" ]
then
  size=`wc -c < $filename`

  if [ $size != 428 ]
  then
    echo $filename changed - should be 428 bytes, not $size bytes
  fi

  chmod 666 $filename
fi

# ---------- file actions.c ----------

filename="actions.c"

if [ -f $filename ]
then
  echo File \"$filename\" already exists\!  Skipping...
  filename=/dev/null		# throw it away
else
  echo extracting file actions.c...
fi

cat << 'END-OF-FILE' > $filename
/**			actions.c			**/

/** RESULT oriented routines *chuckle*.  These routines implement the
    actions that result from either a specified rule being true or from
    the default action being taken.

    (C) Copyright 1986, Dave Taylor
**/

#include <stdio.h>
#include <pwd.h>
#include <ctype.h>
#include <fcntl.h>

#include "defs.h"
#include "filter.h"

FILE *emergency_local_delivery();

mail_message(address)
char *address;
{
	/** Called with an address to send mail to.   For various reasons
	    that are too disgusting to go into herein, we're going to actually
	    open the users mailbox and by hand add this message.  Yech.
	    NOTE, of course, that if we're going to MAIL the message to someone
	    else, that we'll try to do nice things with it on the fly...
	**/

	FILE *pipefd, *tempfd, *mailfd;
	int  in_header = TRUE, line_count = 0;
	char tempfile[SLEN], mailbox[SLEN], lockfile[SLEN],
	     buffer[VERY_LONG_STRING];

	if (verbose && ! log_actions_only && outfd != NULL)
	  fprintf(outfd, "filter (%s): Mailing message to %s\n", 
		   username, address);

	if (! show_only) {
	  sprintf(tempfile, "%s.%d", filter_temp, getpid());

	  if ((tempfd = fopen(tempfile, "r")) == NULL) {
	    if (outfd != NULL)
	      fprintf(outfd, "filter (%s): Can't open temp file %s!!\n", 
		    username, tempfile);
	    if (outfd != NULL) fclose(outfd);
	    exit(1);
	  }
	 	
	  if (strcmp(address, username) != 0) {	/* mailing to someone else */
	    
	    if (already_been_forwarded) {	/* potential looping! */
	      if (contains(from, username)) {
		if (outfd != NULL)
	          fprintf(outfd, 
	"filter (%s): Filter loop detected!  Message left in file %s.%d\n", 
			username, filter_temp, getpid());
	        if (outfd != NULL) fclose(outfd);
	        exit(0);
	      }
	    }

	    sprintf(buffer, "%s %s %s", sendmail, smflags, address);

	    if ((pipefd = popen(buffer, "w")) == NULL) {
	      if (outfd != NULL)
	        fprintf(outfd, "filter (%s): popen %s failed!\n", buffer);
	      sprintf(buffer, "((%s %s %s ; %s %s) & ) < %s &",
		      sendmail , smflags, address, remove, tempfile, tempfile);
	      system(buffer);
	      return;
	    }

	    fprintf(pipefd, "Subject: \"%s\"\n", subject);
	    fprintf(pipefd, "From: The Filter of %s@%s <%s>\n", 
		    username, hostname, username);
	    fprintf(pipefd, "To: %s\n", address);
	    fprintf(pipefd, "X-Filtered-By: filter, version %s\n\n", VERSION);

	    fprintf(pipefd, "-- Begin filtered message --\n\n");
	
	    while (fgets(buffer, LONG_SLEN, tempfd) != NULL)
	      if (already_been_forwarded && in_header)
	        in_header = (strlen(buffer) == 1? 0 : in_header);
	      else
	        fprintf(pipefd," %s", buffer);

	    fprintf(pipefd, "\n-- End of filtered message --\n");
	    fclose(pipefd);
	    fclose(tempfd);
	
	    return;		/* YEAH!  Wot a slick program, eh? */
	  
	  }
	  
	  /** OTHERWISE it is to the current user... **/

	  sprintf(mailbox,  "%s%s", mailhome, username);
	  
	  if (! lock()) {
	    if (outfd != NULL) {
	      fprintf(outfd, "filter (%s): Couldn't create lockfile %s\n",
		    username, lockfile);
	      fprintf(outfd, "filter (%s): Can't open mailbox %s!\n",
			username, mailbox);
	    }
	    if ((mailfd = emergency_local_delivery()) == NULL)
	      exit(1);
	  }
	  else if ((mailfd = fopen(mailbox,"a")) == NULL)
	    if ((mailfd = emergency_local_delivery()) == NULL)
	      exit(1);

	  while (fgets(buffer, sizeof(buffer), tempfd) != NULL) {
	    line_count++;
	    if (the_same(buffer, "From ") && line_count > 1)
	      fprintf(mailfd, ">%s", buffer);
	    else
	      fputs(buffer, mailfd);
	  }

	  fputs("\n", mailfd);

	  fclose(mailfd);
	  unlock();		/* blamo or not?  Let it decide! */
	  fclose(tempfd);
	} /* end if show only */
}

save_message(foldername)
char *foldername;
{
	/** Save the message in a folder.  Use full file buffering to
	    make this work without contention problems **/

	FILE  *fd, *tempfd;
	char  filename[SLEN], buffer[LONG_SLEN];

	if (verbose && outfd != NULL)
	  fprintf(outfd, "filter (%s): Message saved in folder %s\n", 
		  username, foldername);
	
	if (!show_only) {
	  sprintf(filename, "%s.%d", filter_temp, getpid());

	  if ((fd = fopen(foldername, "a")) == NULL) {
	    if (outfd != NULL)
	      fprintf(outfd, 
		 "filter (%s): can't save message to requested folder %s!\n",
		    username, foldername);
	    return(1);
	  }

	  if ((tempfd = fopen(filename, "r")) == NULL) {
	    if (outfd != NULL)
	      fprintf(outfd, 
		     "filter (%s): can't open temp file for reading!\n",
		     username);
	     return(1);
	  }

	  while (fgets(buffer, sizeof(buffer), tempfd) != NULL)
	    fputs(buffer, fd);
	
	  fclose(fd);
	  fclose(tempfd);
	}

 	return(0);
}

execute(command)
char *command;
{
	/** execute the indicated command, feeding as standard input the
	    message we have.
	**/

	char buffer[LONG_SLEN];

	if (verbose && outfd != NULL)
	  fprintf(outfd, "filter (%s): Executing %s\n", 
		  username, command);

	if (! show_only) {
	  sprintf(buffer, "%s %s.%d | %s", cat, filter_temp, getpid(), command);
	  system(buffer);
	}
}

FILE *
emergency_local_delivery()
{
	/** This is called when we can't deliver the mail to the usual
	    mailbox in the usual way ...
	**/

	FILE *tempfd;
	char  mailbox[SLEN];

	sprintf(mailbox, "%s/%s", home, EMERGENCY_MAILBOX);

	if ((tempfd = fopen(mailbox, "a")) == NULL) {
	  if (outfd != NULL)
	    fprintf(outfd, "filter (%s): Can't open %s either!!\n",
		    username, mailbox);

	  sprintf(mailbox,"%s/%s", home, EMERG_MBOX); 

	  if ((tempfd = fopen(mailbox, "a")) == NULL) {

	    if (outfd != NULL) {
	      fprintf(outfd,"filter (%s): Can't open %s either!!!!\n",
		      username, mailbox);
	      fprintf(outfd, 
		      "filter (%s): I can't open ANY mailboxes!  Augh!!\n",
		       username);
	     }

	     fclose(tempfd);
	     leave("Cannot open any mailbox");		/* DIE DIE DIE DIE!! */
	   }
	   else
	     if (outfd != NULL)
	       fprintf(outfd, "filter (%s): Using %s as emergency mailbox\n",
		       username, mailbox);
	  }
	  else
	    if (outfd != NULL)
	      fprintf(outfd, "filter (%s): Using %s as emergency mailbox\n",
		      username, mailbox);

	return((FILE *) tempfd);
}
END-OF-FILE

if [ "$filename" != "/dev/null" ]
then
  size=`wc -c < $filename`

  if [ $size != 6227 ]
  then
    echo $filename changed - should be 6227 bytes, not $size bytes
  fi

  chmod 666 $filename
fi

# ---------- file lock.c ----------

filename="lock.c"

if [ -f $filename ]
then
  echo File \"$filename\" already exists\!  Skipping...
  filename=/dev/null		# throw it away
else
  echo extracting file lock.c...
fi

cat << 'END-OF-FILE' > $filename
/**				locks.c				**/

/** The lock() and unlock() routines herein duplicate exactly the
    equivalent routines in the Elm Mail System, and should also be
    compatible with sendmail, rmail, etc etc.

    (C) Copyright 1986, 1987, 1988, Dave Taylor
**/

#include <stdio.h>
#include <fcntl.h>

#include "defs.h"
#include "filter.h"

int  we_locked_it = 0;
char lockfile[SLEN];

int
lock()
{
	/** This routine will return 1 if we could lock the mailfile,
	    zero otherwise.
	**/

	int attempts = 0, ret;

	sprintf(lockfile,  "%s%s.lock", mailhome, username);

	while ((ret = open(lockfile, O_WRONLY | O_CREAT | O_EXCL, 0777)) < 0 
	       && attempts++ < 10) {
	  sleep(3);	/* wait three seconds each pass, okay?? */
	}

	if (ret > 0) {
	  we_locked_it++;
	  close(ret);			/* no need to keep it open! */
	}

	return( (ret >= 0) );
}

unlock()
{
	/** this routine will remove the lock file, but only if we were
	    the people that locked it in the first place... **/

	if (we_locked_it)
	  unlink(lockfile);	/* blamo! */

	we_locked_it = 0;
}
END-OF-FILE

if [ "$filename" != "/dev/null" ]
then
  size=`wc -c < $filename`

  if [ $size != 1045 ]
  then
    echo $filename changed - should be 1045 bytes, not $size bytes
  fi

  chmod 666 $filename
fi

echo done

exit 0