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;
}