Subject: Re: CRITICAL ** Holes in default cron jobs ** CRITICAL
To: der Mouse <mouse@Holo.Rodents.Montreal.QC.CA>
From: Tim Newsham <newsham@aloha.net>
List: current-users
Date: 01/04/1997 05:54:14
Hi,
I believe I have a solution for the find/rm race condition.
I think that if files are removed without following any symbolic
links then the race between finding and removing cannot be
used to remove arbitrary files. (Is this incorrect?)
I include code below that implements this. Please let me
know if you notice any flaws in the code or the reasoning.
The problem is a bit tricky in that not all symbolic links
should be forbidden. For example /tmp could be a symbolic
link itself. This is accomodated below with the "-r" option
which specifies a root to start walking paths from.
Tim N.
---- src code below ----
/*
* saferm.c
* Safely remove files
*
* saferm removes files in a conservative manner so
* that it cannot be tricked into erasing a file that it
* shouldn't erase. In particular if an intermediate path
* component is a symbolic link then then the file is not
* removed.
*
* Options:
* -r rootdir prefix rootdir to each path. rootdir is
* not checked for symlinks.
*
* Example:
* cd /tmp; find . -type f -atime +3 -print0 | xargs -0 saferm -r /tmp
*/
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/param.h>
#include <sys/stat.h>
/* options */
char *root = "/";
/*
* safechdir
* Change directories in a "safe" way
*
* Change into a directory only if it is not a symbolic link
*/
int
safechdir(char *dir)
{
struct stat sbuf1, sbuf2;
int fd, res;
/* fail if dir is a symlink or non-existant */
res = lstat(dir, &sbuf1);
if(res == -1 || S_ISLNK(sbuf1.st_mode))
return -1;
/* open the dir and verify that it is the same dir */
fd = open(dir, O_RDONLY);
if(fd == -1)
return -1;
res = fstat(fd, &sbuf2);
if(res == -1 || sbuf1.st_dev != sbuf2.st_dev ||
sbuf1.st_ino != sbuf2.st_ino) {
close(fd);
return -1;
}
/* change directories */
res = fchdir(fd);
close(fd);
return res;
}
/*
* saferm
* Remove files in a "safe" way
*
* symbolic links are not followed for intermediate path components
*/
int
saferm(char *file)
{
char filebuf[MAXPATHLEN + 1], *x, *nextcomp;
/* start at our root */
if(chdir(root) == -1) {
fprintf(stderr, "Can't change directories to %s\n", root);
return -1;
}
if(strlen(file) > MAXPATHLEN) {
fprintf(stderr, "%s: filename too long\n", file);
return -1;
}
strcpy(filebuf, file);
/* walk path without following symlinks */
nextcomp = strtok(filebuf, "/");
while((x = strtok(0, "/")) != 0) {
if(x[0] != '\0') {
if(safechdir(nextcomp) == -1) {
fprintf(stderr, "can't safely change directory to %s\n", file);
return -1;
}
}
nextcomp = x;
}
/* unlink the file (which may be a symlink) */
if(unlink(nextcomp) == -1) {
perror(file);
return -1;
}
return 0;
}
void
usage(char *name)
{
fprintf(stderr, "usage:\t%s [-r rootdir] file [files]\n", name);
exit(1);
}
int
main(int argc, char **argv)
{
extern int optind;
extern char *optarg;
int i, res, errs;
char c, *name;
/* parse arguments */
name = argv[0];
while((c = getopt(argc, argv, "r:")) != -1) {
switch(c) {
case 'r':
root = optarg;
break;
default:
usage(name);
}
}
argc -= optind;
argv += optind;
if(argc < 1)
usage(name);
/* unlink each file */
for(; *argv; argv++) {
res = saferm(*argv);
errs += (res == -1);
}
return (errs == 0) ? 0 : 1;
}