Re: Symlinks and Cryogenic Sleep

[marc () SUSE DE (Marc Heuse)]

Hi,

> when you're dealing with files in /tmp that are supposed to be re-opened
> (rather than opened once and then discarded) there's an established
> way to do it which goes like this:
>
>        if (lstat(fname, &stb1) >= 0 && S_ISREG(stb1.st_mode)) {
>                fd = open(fname, O_RDWR);
>                if (fd < 0 || fstat(fd, &stb2) < 0
>                 || ino_or_dev_mismatch(&stb1, &stb2))
>                        raise_big_stink()
>        } else {
>                /* do the O_EXCL thing */
>        }
>
> Accepted wisdom has it that this protects you from symlink attacks.

yes, but not from hardlink attacks

> When trying to explain this to someone, I noticed that this is not quite
> what it does. It protects your application against symlinks to files
> that exist *before the call to lstat*.
> This sounds like I'm nitpicking, and my first reaction also was to try
> to find some convincing handwaving argument to dispel my concerns.

no, thats right, there is still a small race condition.
Thats the reason why O_NOFOLLOW was added to the kernel (bsd and linux >= 2.1.126)

> -      All symlink attacks can be improved by running them on an
>        NFS mounted directory (easy for applications that heed
>        the TMPDIR environment variable). In terms of file system
>        race conditions, NFS acts as a kind of slo-mo glue.

however, who is mounting /tmp via NFS? rare (can be found sometimes, yes)
And it's always a good idea to make /tmp a partition of it's own.
(and Theo de Raadt even goes that far to set quotas on /tmp ... ;-)

> -      It's not that hard to detect whether the targetted application is
>        in the critical section of code. On Linux, it's very easy because
>        /proc/$pid/stat will give you the instruction pointer. On other OSes

wasn't that fixed? Thomas (thomas () suse de) found that vulnerability and
reported it. As far as I know it's fixed (hmm, but I'm not sure).

> Comments? Suggestions?

hmm, I'd propose the following function to protect against the hard/symlink
attacks (add syslog/warnings wherever you want to). Comments welcome.

#ifndef O_NOFOLLOW
# define O_NOFOLLOW 0  // was introduced in kernel 2.1.126, needs glibc > 2.0.100
#endif

int safe_reopen (char *file, int modes) {
    struct stat st;
    struct stat st2;
    int fd;

    if (lstat(TMPFILE, &st) < 0) { // does not exit -> safe creation
        if ((fd=open(TMPFILE, modes | O_EXCL | O_CREAT, 0600)) < 0)
            return -1;
    } else { // it exists - allow only regular file which are not hardlinked
        if ((! S_ISREG(st.st_mode)) || st.st_nlink != 1)
            return -1; // OK, lets open
        if ((fd=open(TMPFILE, modes | O_NOFOLLOW)) < 0)
            return -1;
        fstat(fd, &st2); // recheck that it's the same file ...
        if (st2.st_ino != st.st_ino || st2.st_uid != st.st_uid || st2.st_nlink != 1) {
            close(fd);
            return -1;
        }
    }
    return fd;
}

Greets,
        Marc

--
   Marc Heuse, SuSE GmbH, Schanzaeckerstr. 10, 90443 Nuernberg
   E@mail: marc () suse de  Function: Security Support & Auditing
   "lynx -source http://www.suse.de/~marc/marc.pgp | pgp -fka"
Key fingerprint = B5 07 B6 4E 9C EF 27 EE  16 D9 70 D4 87 B5 63 6C