Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 13 Apr 1995 15:17:00 +0300 (GMT+0300)
From:      adam <adam@math.tau.ac.il>
To:        freebsd-security@FreeBSD.org
Subject:   Re: cvs commit: src/usr.sbin/cron/cron do_command.c
Message-ID:  <199504131217.PAA21237@lune.math.tau.ac.il>
In-Reply-To: <199504122010.PAA03812@mpp.com> from "Mike Pritchard" at Apr 12, 95 03:10:12 pm

next in thread | previous in thread | raw e-mail | index | archive | help
> >   Modified:    usr.sbin/cron/cron do_command.c
> >   Log:
> >   Close MAILTO security hole
> I took a look at your fix, and the security hole is still there.  Simply 
> checking if the first character of the MAILTO variable is a '-' isn't 
> enough, since I could simply prefix the MAILTO variable with a space (or 
> lots of them or whatever).  I can also add additional arguments,
> which with sendmail isn't a problem, but what if the administrator chooses
> to edit cron/config.h and use a different mail delivery program?
> Then who knows how those extra arguments are going to be used.

The cron in question is vixie-cron-3.0.  I emailed Paul Vixie around
the time I posted the hole in Thomas Koenig's atrun, and also included
a patch.  Since that cron hasn't been updated for quite some time, and
is probably not at the top of his list, it's taking a while, though
I'm only guessing (and it hasn't really been such a long while).

My suggestion is not to run Sendmail as root.  If you want, you can
``verify'' MAILTO, but IMHO, such a fix is begging to fail, because
you need to start studying Sendmail and seeing what wrongs it can do
running as root.  Like, the obvious fix of searching for ``-'' fails
for people who mail ``cron-people''.

Running sendmail as the user seems to work fine, including From: headers
and everything.

Well, it's out... so here's my patch.  safe_p() is a slight modification
of something Paul Vixie wrote.

*** do_command.c.orig	Sun Apr  9 18:29:18 1995
--- do_command.c	Sun Apr  9 19:47:34 1995
***************
*** 33,38 ****
--- 33,39 ----
  static void		child_process __P((entry *, user *)),
  			do_univ __P((user *));
  
+ static int		safe_p __P((const char *));
  
  void
  do_command(e, u)
***************
*** 360,365 ****
--- 361,369 ----
  			 * up the mail command and subjects and stuff...
  			 */
  
+ 			if (!safe_p(mailto)) 
+ 				log_it(usernm, getpid(), "UNSAFE", mailto);
+ 
  			if (mailto) {
  				register char	**env;
  				auto char	mailcmd[MAX_COMMAND];
***************
*** 368,373 ****
--- 372,383 ----
  				(void) gethostname(hostname, MAXHOSTNAMELEN);
  				(void) sprintf(mailcmd, MAILARGS, MAILCMD, mailto);
  
+ 				setgid(e->gid);
+ #if defined (BSD)
+ 				initgroups(env_get("LOGNAME", e->envp), e->gid);
+ #endif
+ 				setuid(e->uid);
+ 
  				if (!(mail = cron_popen(mailcmd, "w"))) {
  					perror(MAILCMD);
  					(void) _exit(ERROR_EXIT);
***************
*** 462,467 ****
--- 472,492 ----
  	}
  }
  
+ static int
+ safe_p(s)
+ 	register const char *s;
+ {
+ 	static const char safe_delim[] = "@!:%-_.";
+ 	register char ch;
+ 
+ 	while ((ch = *s++) != '\0') {
+ 		if (isascii(ch) && isprint(ch) && 
+ 		    (isalnum(ch) || strchr(safe_delim, ch)))
+ 			continue;
+ 		return (FALSE);
+ 	}
+ 	return (TRUE);
+ }
  
  static void
  do_univ(u)


adam?




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