Skip to content

WIP SLOP use descriptors to avoid TOCTOU for canonicalising file system meta data #15120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Ericson2314 wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

WIP SLOP use descriptors to avoid TOCTOU for canonicalising file system meta data #15120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe43974
Remove suppport for not having `lchown`
Ericson2314 Jan 30, 2026
288b77e
WIP SLOP use descriptors to avoid TOCTOU for canonicalising file syst…
Ericson2314 Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libexpr/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ deps_public_maybe_subproject = [
subdir('nix-meson-build-support/subprojects')
subdir('nix-meson-build-support/big-objs')

# Check for each of these functions, and create a define like `#define HAVE_LCHOWN 1`.
# Check for each of these functions, and create a define like `#define HAVE_SYSCONF 1`.
check_funcs = [
'sysconf',
]
Expand Down
24 changes: 22 additions & 2 deletions src/libfetchers/git.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@
#include "nix/util/json-utils.hh"
#include "nix/util/archive.hh"
#include "nix/util/mounted-source-accessor.hh"
#include "nix/util/file-descriptor.hh"
#include "nix/util/canon-path.hh"

#include <regex>
#include <string.h>
#include <sys/time.h>
#include <fcntl.h>

#ifndef _WIN32
# include <sys/wait.h>
Expand Down Expand Up @@ -846,8 +849,25 @@ struct GitInputScheme : InputScheme
}

try {
if (!input.getRev())
setWriteTime(localRefFile, now, now);
if (!input.getRev()) {
auto parent = localRefFile.parent_path();
auto name = localRefFile.filename();
AutoCloseFD dirFd
#ifndef _WIN32
= toDescriptor(open(parent.string().c_str(), O_RDONLY | O_DIRECTORY | O_CLOEXEC));
#else
= CreateFileW(
parent.c_str(),
GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS,
NULL);
#endif
if (dirFd)
setWriteTime(dirFd.get(), CanonPath(name.string()), now, now);
}
} catch (Error & e) {
warn("could not update mtime for file %s: %s", PathFmt(localRefFile), e.info().msg);
}
Expand Down
4 changes: 1 addition & 3 deletions src/libstore/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ endif
summary('can hardlink to symlink', can_link_symlink, bool_yn : true)
configdata_priv.set('CAN_LINK_SYMLINK', can_link_symlink.to_int())

# Check for each of these functions, and create a define like `#define HAVE_LCHOWN 1`.
# Check for each of these functions, and create a define like `#define HAVE_POSIX_FALLOCATE 1`.
check_funcs = [
# Optionally used for canonicalising files from the build
'lchown',
'posix_fallocate',
'statvfs',
]
Expand Down
133 changes: 86 additions & 47 deletions src/libstore/posix-fs-canonicalise.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,16 @@
#include "nix/util/util.hh"
#include "nix/store/store-api.hh"
#include "nix/store/globals.hh"
#include "nix/util/file-descriptor.hh"
#include "nix/util/canon-path.hh"
#include "store-config-private.hh"

#ifndef _WIN32
# include <fcntl.h>
# include <dirent.h>
# include <sys/stat.h>
#endif

#if NIX_SUPPORT_ACL
# include <sys/xattr.h>
#endif
Expand All @@ -15,76 +23,106 @@ namespace nix {

const time_t mtimeStore = 1; /* 1 second into the epoch */

static void canonicaliseTimestampAndPermissions(const Path & path, const PosixStat & st)
static void canonicaliseTimestampAndPermissions(Descriptor dirFd, const CanonPath & path, const struct stat & st)
{
if (!S_ISLNK(st.st_mode)) {

/* Mask out all type related bits. */
mode_t mode = st.st_mode & ~S_IFMT;
bool isDir = S_ISDIR(st.st_mode);
if ((mode != 0444 || isDir) && mode != 0555) {
mode = (st.st_mode & S_IFMT) | 0444 | (st.st_mode & S_IXUSR || isDir ? 0111 : 0);
chmod(path, mode);
mode = 0444 | (st.st_mode & S_IXUSR || isDir ? 0111 : 0);
#ifndef _WIN32
unix::fchmodatTryNoFollow(dirFd, path, mode);
#else
// TODO: implement fchmodatTryNoFollow for Windows
#endif
}
}

#ifndef _WIN32 // TODO implement
if (st.st_mtime != mtimeStore) {
PosixStat st2 = st;
st2.st_mtime = mtimeStore, setWriteTime(path, st2);
setWriteTime(dirFd, path, st.st_atime, mtimeStore);
}
#endif
}

void canonicaliseTimestampAndPermissions(const Path & path)
{
canonicaliseTimestampAndPermissions(path, lstat(path));
auto parent = std::filesystem::path(path).parent_path();
auto name = std::filesystem::path(path).filename();

if (parent.empty())
parent = ".";

AutoCloseFD dirFd = toDescriptor(open(parent.string().c_str(), O_RDONLY | O_DIRECTORY | O_CLOEXEC));
if (!dirFd)
throw SysError("opening parent directory of '%s'", path);

struct stat st;
if (fstatat(dirFd.get(), name.string().c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1)
throw SysError("statting '%s'", path);

canonicaliseTimestampAndPermissions(dirFd.get(), CanonPath(name.string()), st);
}

static void
canonicalisePathMetaData_(const Path & path, CanonicalizePathMetadataOptions options, InodesSeen & inodesSeen)
static void canonicalisePathMetaData_(
Descriptor dirFd, const CanonPath & path, CanonicalizePathMetadataOptions options, InodesSeen & inodesSeen)
{
checkInterrupt();

struct stat st;
if (fstatat(dirFd, path.rel_c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1)
throw SysError("statting '%s'", path);

#ifdef __APPLE__
/* Remove flags, in particular UF_IMMUTABLE which would prevent
the file from being garbage-collected. FIXME: Use
setattrlist() to remove other attributes as well. */
if (lchflags(path.c_str(), 0)) {
AutoCloseFD fd = openat(dirFd, path.rel_c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC);
if (!fd)
throw SysError("opening '%s' to clear flags", path);
if (fchflags(fd.get(), 0)) {
if (errno != ENOTSUP)
throw SysError("clearing flags of path '%1%'", path);
}
#endif

auto st = lstat(path);

/* Really make sure that the path is of a supported type. */
if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)))
throw Error("file '%1%' has an unsupported type", path);

#if NIX_SUPPORT_ACL
/* Remove extended attributes / ACLs. */
ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0);
/* We need a file descriptor for xattr operations on Linux. */
AutoCloseFD fd = openat(
dirFd,
path.rel_c_str(),
O_RDONLY | O_NOFOLLOW | O_CLOEXEC
# ifdef O_PATH
| O_PATH
# endif
);
if (!fd)
throw SysError("opening '%s' to remove extended attributes", path);

ssize_t eaSize = flistxattr(fd.get(), nullptr, 0);

if (eaSize < 0) {
if (errno != ENOTSUP && errno != ENODATA)
throw SysError("querying extended attributes of '%s'", path);
} else if (eaSize > 0) {
std::vector<char> eaBuf(eaSize);

if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0)
if ((eaSize = flistxattr(fd.get(), eaBuf.data(), eaBuf.size())) < 0)
throw SysError("querying extended attributes of '%s'", path);

for (auto & eaName : tokenizeString<Strings>(std::string(eaBuf.data(), eaSize), std::string("\000", 1))) {
if (options.ignoredAcls.count(eaName))
continue;
if (lremovexattr(path.c_str(), eaName.c_str()) == -1)
if (fremovexattr(fd.get(), eaName.c_str()) == -1)
throw SysError("removing extended attribute '%s' from '%s'", eaName, path);
}
}
#endif

#ifndef _WIN32
/* Fail if the file is not owned by the build user. This prevents
us from messing up the ownership/permissions of files
hard-linked into the output (e.g. "ln /etc/shadow $out/foo").
Expand All @@ -100,58 +138,59 @@ canonicalisePathMetaData_(const Path & path, CanonicalizePathMetadataOptions opt
|| (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore));
return;
}
#endif

inodesSeen.insert(Inode(st.st_dev, st.st_ino));

canonicaliseTimestampAndPermissions(path, st);
canonicaliseTimestampAndPermissions(dirFd, path, st);

#ifndef _WIN32
/* Change ownership to the current uid. If it's a symlink, use
lchown if available, otherwise don't bother. Wrong ownership
of a symlink doesn't matter, since the owning user can't change
the symlink and can't delete it because the directory is not
writable. The only exception is top-level paths in the Nix
store (since that directory is group-writable for the Nix build
users group); we check for this case below. */
/* Change ownership to the current uid. */
if (st.st_uid != geteuid()) {
# if HAVE_LCHOWN
if (lchown(path.c_str(), geteuid(), getegid()) == -1)
# else
if (!S_ISLNK(st.st_mode) && chown(path.c_str(), geteuid(), getegid()) == -1)
# endif
if (fchownat(dirFd, path.rel_c_str(), geteuid(), getegid(), AT_SYMLINK_NOFOLLOW) == -1)
throw SysError("changing owner of '%1%' to %2%", path, geteuid());
}
#endif

if (S_ISDIR(st.st_mode)) {
for (auto & i : DirectoryIterator{path}) {
AutoCloseFD childDirFd =
openFileEnsureBeneathNoSymlinks(dirFd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
if (!childDirFd)
throw SysError("opening directory '%s'", path);

AutoCloseDir dir(fdopendir(dup(childDirFd.get())));
if (!dir)
throw SysError("opening directory '%s' for iteration", path);

struct dirent * dirent;
while (errno = 0, dirent = readdir(dir.get())) {
checkInterrupt();
canonicalisePathMetaData_(i.path().string(), options, inodesSeen);
std::string childName = dirent->d_name;
if (childName == "." || childName == "..")
continue;
canonicalisePathMetaData_(childDirFd.get(), CanonPath(childName), options, inodesSeen);
}
if (errno)
throw SysError("reading directory '%s'", path);
}
}

void canonicalisePathMetaData(const Path & path, CanonicalizePathMetadataOptions options, InodesSeen & inodesSeen)
{
canonicalisePathMetaData_(path, options, inodesSeen);
auto parent = std::filesystem::path(path).parent_path();
auto name = std::filesystem::path(path).filename();

#ifndef _WIN32
/* On platforms that don't have lchown(), the top-level path can't
be a symlink, since we can't change its ownership. */
auto st = lstat(path);
if (parent.empty())
parent = ".";

if (st.st_uid != geteuid()) {
assert(S_ISLNK(st.st_mode));
throw Error("wrong ownership of top-level store path '%1%'", path);
}
#endif
AutoCloseFD dirFd = toDescriptor(open(parent.string().c_str(), O_RDONLY | O_DIRECTORY | O_CLOEXEC));
if (!dirFd)
throw SysError("opening parent directory of '%s'", path);

canonicalisePathMetaData_(dirFd.get(), CanonPath(name.string()), options, inodesSeen);
}

void canonicalisePathMetaData(const Path & path, CanonicalizePathMetadataOptions options)
{
InodesSeen inodesSeen;
canonicalisePathMetaData_(path, options, inodesSeen);
canonicalisePathMetaData(path, options, inodesSeen);
}

} // namespace nix
7 changes: 1 addition & 6 deletions src/libutil/file-system.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,11 +676,6 @@ void replaceSymlink(const std::filesystem::path & target, const std::filesystem:
}
}

void setWriteTime(const std::filesystem::path & path, const PosixStat & st)
{
setWriteTime(path, st.st_atime, st.st_mtime, S_ISLNK(st.st_mode));
}

void copyFile(const std::filesystem::path & from, const std::filesystem::path & to, bool andDelete)
{
auto fromStatus = std::filesystem::symlink_status(from);
Expand All @@ -705,7 +700,7 @@ void copyFile(const std::filesystem::path & from, const std::filesystem::path &
throw Error("file %s has an unsupported type", PathFmt(from));
}

setWriteTime(to, lstat(from.string().c_str()));
// Note: we don't preserve timestamps anymore
if (andDelete) {
if (!std::filesystem::is_symlink(fromStatus))
std::filesystem::permissions(
Expand Down
8 changes: 8 additions & 0 deletions src/libutil/include/nix/util/file-descriptor.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,14 @@ Descriptor openFileEnsureBeneathNoSymlinks(
#endif
);

/**
* Set the access and modification time of a file relative to a directory file descriptor.
*
* @pre path.isRoot() is false
* @throws SysError if any operation fails
*/
void setWriteTime(Descriptor dirFd, const CanonPath & path, time_t accessedTime, time_t modificationTime);

#ifndef _WIN32
namespace unix {

Expand Down
24 changes: 0 additions & 24 deletions src/libutil/include/nix/util/file-system.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,30 +299,6 @@ void createDirs(const std::filesystem::path & path);
*/
void createDir(const Path & path, mode_t mode = 0755);

/**
* Set the access and modification times of the given path, not
* following symlinks.
*
* @param accessedTime Specified in seconds.
*
* @param modificationTime Specified in seconds.
*
* @param isSymlink Whether the file in question is a symlink. Used for
* fallback code where we don't have `lutimes` or similar. if
* `std::optional` is passed, the information will be recomputed if it
* is needed. Race conditions are possible so be careful!
*/
void setWriteTime(
const std::filesystem::path & path,
time_t accessedTime,
time_t modificationTime,
std::optional<bool> isSymlink = std::nullopt);

/**
* Convenience wrapper that takes all arguments from the `PosixStat`.
*/
void setWriteTime(const std::filesystem::path & path, const PosixStat & st);

/**
* Create a symlink.
*
Expand Down
27 changes: 27 additions & 0 deletions src/libutil/unix/file-descriptor.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,4 +410,31 @@ OsString readLinkAt(Descriptor dirFd, const CanonPath & path)
}
}

void setWriteTime(Descriptor dirFd, const CanonPath & path, time_t accessedTime, time_t modificationTime)
{
struct timespec times[2] = {
{
.tv_sec = accessedTime,
.tv_nsec = 0,
},
{
.tv_sec = modificationTime,
.tv_nsec = 0,
},
};

#if HAVE_UTIMENSAT && HAVE_DECL_AT_SYMLINK_NOFOLLOW
if (utimensat(dirFd, path.rel_c_str(), times, AT_SYMLINK_NOFOLLOW) == -1)
throw SysError("changing modification time of '%s' (using `utimensat`)", path);
#else
// Fallback: open the file and use futimens
AutoCloseFD fd = openat(dirFd, path.rel_c_str(), O_RDONLY | O_NOFOLLOW | O_CLOEXEC);
if (!fd)
throw SysError("opening '%s' to change modification time", path);

if (futimens(fd.get(), times) == -1)
throw SysError("changing modification time of '%s' (using `futimens`)", path);
#endif
}

} // namespace nix
Loading
Loading