From owner-freebsd-security Thu Apr 13 05:19:29 1995 Return-Path: security-owner Received: (from majordom@localhost) by freefall.cdrom.com (8.6.10/8.6.6) id FAA12302 for security-outgoing; Thu, 13 Apr 1995 05:19:29 -0700 Received: from taurus.math.tau.ac.il (root@taurus.math.tau.ac.il [132.67.64.4]) by freefall.cdrom.com (8.6.10/8.6.6) with ESMTP id FAA12294 for ; Thu, 13 Apr 1995 05:19:13 -0700 Received: from lune.math.tau.ac.il (adam@lune.math.tau.ac.il [132.67.96.11]) by taurus.math.tau.ac.il (8.6.10/8.6.10) with ESMTP id PAA20970; Thu, 13 Apr 1995 15:17:01 +0300 From: adam Received: (adam@localhost) by lune.math.tau.ac.il (8.6.9/8.6.9) id PAA21237; Thu, 13 Apr 1995 15:17:00 +0300 Message-Id: <199504131217.PAA21237@lune.math.tau.ac.il> Subject: Re: cvs commit: src/usr.sbin/cron/cron do_command.c To: freebsd-security@FreeBSD.org Date: Thu, 13 Apr 1995 15:17:00 +0300 (GMT+0300) In-Reply-To: <199504122010.PAA03812@mpp.com> from "Mike Pritchard" at Apr 12, 95 03:10:12 pm X-Sender: adam@math.tau.ac.il X-Organization: DIS WHEEL SHALL EXPL0DE X-Mailer: ELM [version 2.4 PL24] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Length: 2829 Sender: security-owner@FreeBSD.org Precedence: bulk > > 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?