*BSD News Article 25176


Return to BSD News archive

Newsgroups: comp.os.386bsd.bugs
Path: sserve!newshost.anu.edu.au!munnari.oz.au!news.Hawaii.Edu!ames!elroy.jpl.nasa.gov!grian!puffin!pete
From: pete@puffin.uucp (Pete Carah)
Subject: Re: [NetBSD V0.9] Crontab Security Problem
References: <9312171222.AA01518@fee.unicamp.br> <CI76zM.7qw@Colorado.EDU> <MARK_WEAVER.93Dec18202545@localhost.cs.brown.edu>
Organization: /usr/lib/news/organi[sz]ation
Date: Sun, 19 Dec 1993 09:11:57 GMT
Message-ID: <CI9yvx.CIJ@puffin.uucp>
Lines: 45

In article <MARK_WEAVER.93Dec18202545@localhost.cs.brown.edu>,
 <Mark_Weaver@brown.edu> wrote:
>In article <CI76zM.7qw@Colorado.EDU> Todd C. Miller <millert@cs.Colorado.EDU> writes:
>> Here's the fix I use.  I had a nicer patch that I wrote but seem to have
>> nuked it during directory cleanup :-(
>> 
>> *** crontab.c	Wed Jul 18 01:23:57 1990
>> --- ../../cron-2.1/crontab.c	Tue Sep 14 19:34:10 1993
>> ***************
>> *** 207,216 ****
>> --- 205,217 ----
>>   		if (!strcmp(Filename, "-")) {
>>   			NewCrontab = stdin;
>>   		} else {
>> + 			/* swap effective/real uid to plug security hole */
>> + 			setreuid(geteuid(), getuid());
>>   			if (!(NewCrontab = fopen(Filename, "r"))) {
>>   				perror(Filename);
>>   				exit(ERROR_EXIT);
>>   			}
>> + 			setreuid(getuid(), geteuid());
>>   		}
>>   	}
>> -- 
>>   Todd C. Miller    Sysadmin--University of Colorado    millert@cs.Colorado.EDU
>
>This patch is broken.  That second call to setreuid should be exactly
>the same as the first.  Right now, that second call isn't doing

Also, an easier fix is:
                if (!strcmp(Filename, "-")) {
                        NewCrontab = stdin;
                } else {
!                       if (access(Filename, R_OK) < 0 ||
!                           !(NewCrontab = fopen(Filename, "r"))) {
                                perror(Filename);
                                exit(ERROR_EXIT);
                        }
                }
--------------
access(2) uses the REuid for checks; it was meant for exactly this use.
We don't have to check errno for permissions; it doesn't matter why
access(2) fails.

-- Pete