[PATCH] Fix potential filesystem race in crontab

Ryan Mallon rmallon at gmail.com
Wed Oct 2 21:39:49 UTC 2013


On 02/10/13 18:01, Tito wrote:

> Hi,
> just my 2 cents:
> 

<snip>

>>  static int open_as_user(const struct passwd *pas, const char *file)
>>  {
>> -	pid_t pid;
>> -	char c;
>> -
>> -	pid = xvfork();
>> -	if (pid) { /* PARENT */
>> -		if (wait4pid(pid) == 0) {
>> -			/* exitcode 0: child says it can read */
>> -			return open(file, O_RDONLY);
>> -		}
>> -		return -1;
>> -	}
>> -
>> -	/* CHILD */
>> -	/* initgroups, setgid, setuid */
>> -	change_identity(pas);
>> -	/* We just try to read one byte. If it works, file is readable
>> -	 * under this user. We signal that by exiting with 0. */
>> -	_exit(safe_read(xopen(file, O_RDONLY), &c, 1) < 0);
>> +	uid_t old_euid;
>> +	gid_t old_egid;
>> +	int fd, err;
>> +
>> +	/* Save and drop privileges */
>> +	old_euid = geteuid();
>> +	old_egid = getegid();
> 
> Making this a function could eventually reduce size.
> 
> change_privileges(uid_t uid, gid_t gid)
> {
> 	if ( setegid(gid) < 0 || seteuid(uid) < 0)
> 		bb_perror_msg_and_die("failed to set egid=%d  euid=%d ", gid, uid);
> }


Yeah, agreed. I wanted to get feedback on the approach first to make
sure that it is acceptable.

~Ryan



More information about the busybox mailing list