Skip to content

Using Path::is_dir() leads to many TOCTOU races. #9450

@collinfunk

Description

@collinfunk

I mentioned in #9342 that a TOCTOU race was introduced. Here is a simplified version of that change to demonstrate the issue:

if src.is_dir() {
  /* Special error message.  */
}
/* At this point, SRC may no longer be a directory.  */
link(src, dest);

Using strace we can see that link is never called. Instead, we fail after a call to statx:

$ C_ALL=C strace uu_ln dir link
[...]
statx(3, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID|STATX_SUBVOL, stx_attributes=0, stx_mode=S_IFREG|0644, stx_size=2278, ...}) = 0
read(3, "ln-about = Make links between fi"..., 2278) = 2278
read(3, "", 32)                         = 0
[...]
write(2, "ln", 2ln)                       = 2
write(2, ": ", 2: )                       = 2
write(2, "dir: hard link not allowed for d"..., 40dir: hard link not allowed for directory) = 40
write(2, "\n", 1
)                       = 1
[...]

This is incorrect, since after the call to statx and before the link, another process may remove src and/or create a file in it's place. If it is removed then the error message is wrong. If a new file is created then the link should be created successfully instead of failing.

Here is the behavior of GNU coreutils:

$ LC_ALL=C strace ln dir link
[...]
linkat(AT_FDCWD, "dir", AT_FDCWD, "link", 0) = -1 EPERM (Operation not permitted)
newfstatat(AT_FDCWD, "dir", {st_mode=S_IFDIR|0755, st_size=0, ...}, AT_SYMLINK_NOFOLLOW) = 0
fcntl(1, F_GETFL)                       = 0x80002 (flags O_RDWR|O_CLOEXEC)
write(2, "ln: ", 4ln: )                     = 4
write(2, "dir: hard link not allowed for d"..., 40dir: hard link not allowed for directory) = 40
write(2, "\n", 1
)                       = 1
[...]

So we do not skip calling link. If it fails, we use a fstat only as an attempt to provide better diagnostics to the user.

The tac program has a similar issue with Path::is_dir(). Here is uutil's behavior:

$ LC_ALL=C strace uu_tac /
statx(AT_FDCWD, "/", AT_STATX_SYNC_AS_STAT, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID|STATX_SUBVOL, stx_attributes=STATX_ATTR_MOUNT_ROOT, stx_mode=S_IFDIR|0555, stx_size=180, ...}) = 0
write(2, "tac", 3tac)                      = 3
write(2, ": ", 2: )                       = 2
write(2, "/: read error: Invalid argument", 31/: read error: Invalid argument) = 31
write(2, "\n", 1
)                       = 1

It is also strange, in my opinion, to say there was a read error without opening a file descriptor and calling read.

Here is GNU's behavior:

$ LC_ALL=C strace tac /
[...]
openat(AT_FDCWD, "/", O_RDONLY)         = 3
lseek(3, 0, SEEK_END)                   = 180
ioctl(3, TCGETS2, 0x7ffc9a9cc750)       = -1 ENOTTY (Inappropriate ioctl for device)
lseek(3, 0, SEEK_SET)                   = 0
read(3, 0x559eaee8e361, 8192)           = -1 EISDIR (Is a directory)
fcntl(1, F_GETFL)                       = 0x80002 (flags O_RDWR|O_CLOEXEC)
write(2, "tac: ", 5tac: )                    = 5
write(2, "/: read error", 13/: read error)           = 13
write(2, ": Is a directory", 16: Is a directory)        = 16
write(2, "\n", 1
)                       = 1
[...]

Here we actually perform a read and trust it's error instead of relying on stat.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions