[security] Directory.Delete(@"somepath", true) can be duped by a malicious process to descend other directories

[joshudson]

I have inspected the Windows and the Linux code and found both to be not armored against directory traversal switcheroo.

Exploit path:

1. Create a folder in the root directory with a bunch of files and subdirectories
2. Create a subdirectory with a large tree that mimics the contents of the directory you want the administrator to destroy
3. Wait for the administrator to attempt the delete. This is best done with FileSystemWatcher
4. Swap the subdirectory with a symbolic link to the target.

In the Linux world, the system calls openat() and unlinkat() were added to deal with this specific situation. The shortest correct sequence of system calls would be as follows:

    int RemoveDirectoryTree(const char *path)
    {
            struct statbuf basestat;
            int handle = open(path, 0));
            fstat(handle, &basestat);
            if (RecursiveDestroy(handle, &basestat)) { close(handle); return -1; }
            close(handle);
            return rmdir(path);
    }

    int RecursiveDestroy(int handle, struct statbuf *pstat)
    {
        struct old_linux_dirent de;
        while (sys_readdir(handle, &de, 1)) {
    again:
            int h2 = openat(handle, de.de_name, 0)); /* bombs on certain kinds of nodes returning -1 but we don't have to care */
            int openaterrno = (h2 >= 0) ? 0 : errno;
            if (!(unlinkat(handle, de, 0)) {
                if (errno == EISDIR) {
                    if (h2 < 0 && openaterrno == ENFILE) return -1; // Out of handles
                    if (h2 < 0) goto again; // node changed out from under us -- HANDLE IT
                    struct statbuf newstat;
                    fstat(handle, &newstat);
                    if (newstat.st_dev != pstat->st_dev) {
                        errno = EBUSY; // mount point
                        close(h2);
                        return -1;
                    }
                    if (newstat.st_ino != pstat->st_ino) {
                        close(h2);
                        goto again; // node changed out from under us -- HANDLE IT
                    }
                    if (RecursiveDestroy(handle, newstat)) { close(h2); return -1; }
                } else {
                    close(h2);
                    return -1;
               }
            }
            if (h2 >= 0) close(h2);
        }
        return 0;
    }

There are equivalent calls in the NT Native API to do this on Windows. I have been unable to find any way at all to do this win Win32 calls.

So you might think you can just document to not do this, but Powershell on Linux uses .NET Core and .NET Framework on Windows has exactly the same bug.

[danmoseley]

@JeremyKuhne

[JeremyKuhne]

@morganbr

[krwq]

I think this is reliability not security issue - attacker has to already have IO access to abuse this.

For future reference please follow https://github.com/dotnet/corefx/#reporting-security-issues-and-security-bugs for any potential security issues.

[joshudson]

@krwq: Windows Remote Desktop services thinks IO access is not equivalent: https://docs.microsoft.com/en-us/windows-server/remote/remote-desktop-services/welcome-to-rds

secure@microsoft.com is indistinguishable from a black hole.

[setharnold]

I believe you need to add errno = 0; before if (!(unlinkat(handle, de, 0)).

Thanks

[JeremyKuhne]

The handle relative APIs may make things more speedy, which is precisely why I use them for enumeration on Windows. They also help keep context if the parent directory structure gets modified, so that's a small bonus. I'm perfectly fine with switching to handle relative where possible if it makes things faster, but this doesn't make things atomic. You could just as easily modify a subsequent subdirectory before it's ever touched and we wouldn't be able to determine ultimate intent.

I don't know the handle relative way to delete files on Windows off the top of my head (without using NtCreateFile, which presumably is not what DeleteFile does).

[joshudson]

Somebody adding another file while recursive destroy is running is not a successful exploit. Somebody tricking it into leaving the target tree altogether is. This can only be done with symbolic links.

[JeremyKuhne]

Somebody adding another file while recursive destroy is running is not a successful exploit. Somebody tricking it into leaving the target tree altogether is. This can only be done with symbolic links.

I meant adding another symbolic link. If you have a directory with A, B, C, ... Z entries you can easily replace C with something else while we're working through A & B (or entry 100, etc.). Why not just replace link E with another symbolic link to another location if you already have rights to do so? Why not do it even before you start deleting? And what guarantees that entry Z in a directory isn't going to change to something else at the OS level after it starts enumerating? Or in a subdirectory? Staring a directory enumeration doesn't lock the file system from further changes.

You could also find ways to flip the original file path to a symbolic link as well. In any case, if you still feel that there is a real issue here please follow the given link: https://github.com/dotnet/corefx/#reporting-security-issues-and-security-bugs

[karelz]

Triage: Seems to be answered above. If there is still security concern, please follow the process linked above.