Skip to content

Commit fa380e0

Browse files
committed
libutil: Make RestoreSink use *at system calls on UNIX
This is necessary to ban symlink following. It can be considered a defense in depth against issues similar to CVE-2024-45593. By slightly changing the API in a follow-up commit we will be able to mitigate the symlink following issue for good.
1 parent 533cced commit fa380e0

File tree

2 files changed

+53
-1
lines changed

2 files changed

+53
-1
lines changed

src/libutil/fs-sink.cc

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,34 @@ static std::filesystem::path append(const std::filesystem::path & src, const Can
7373
void RestoreSink::createDirectory(const CanonPath & path)
7474
{
7575
auto p = append(dstPath, path);
76+
77+
#ifndef _WIN32
78+
if (dirFd) {
79+
if (path.isRoot())
80+
/* Trying to create a directory that we already have a file descriptor for. */
81+
throw Error("path '%s' already exists", p.string());
82+
83+
if (::mkdirat(dirFd.get(), path.rel_c_str(), 0777) == -1)
84+
throw SysError("creating directory '%s'", p.string());
85+
86+
return;
87+
}
88+
#endif
89+
7690
if (!std::filesystem::create_directory(p))
7791
throw Error("path '%s' already exists", p.string());
92+
93+
#ifndef _WIN32
94+
if (path.isRoot()) {
95+
assert(!dirFd); // Handled above
96+
97+
/* Open directory for further *at operations relative to the sink root
98+
directory. */
99+
dirFd = open(p.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC);
100+
if (!dirFd)
101+
throw SysError("creating directory '%1%'", p.string());
102+
}
103+
#endif
78104
};
79105

80106
struct RestoreRegularFile : CreateRegularFileSink
@@ -114,7 +140,14 @@ void RestoreSink::createRegularFile(const CanonPath & path, std::function<void(C
114140
FILE_ATTRIBUTE_NORMAL,
115141
NULL)
116142
#else
117-
open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)
143+
[&]() {
144+
/* O_EXCL together with O_CREAT ensures symbolic links in the last
145+
component are not followed. */
146+
constexpr int flags = O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC;
147+
if (!dirFd)
148+
return ::open(p.c_str(), flags, 0666);
149+
return ::openat(dirFd.get(), path.rel_c_str(), flags, 0666);
150+
}();
118151
#endif
119152
;
120153
if (!crf.fd)
@@ -161,6 +194,13 @@ void RestoreRegularFile::operator()(std::string_view data)
161194
void RestoreSink::createSymlink(const CanonPath & path, const std::string & target)
162195
{
163196
auto p = append(dstPath, path);
197+
#ifndef _WIN32
198+
if (dirFd) {
199+
if (::symlinkat(requireCString(target), dirFd.get(), path.rel_c_str()) == -1)
200+
throw SysError("creating symlink from '%1%' -> '%2%'", p.string(), target);
201+
return;
202+
}
203+
#endif
164204
nix::createSymlink(target, p.string());
165205
}
166206

src/libutil/include/nix/util/fs-sink.hh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,18 @@ struct NullFileSystemObjectSink : FileSystemObjectSink
8282
struct RestoreSink : FileSystemObjectSink
8383
{
8484
std::filesystem::path dstPath;
85+
#ifndef _WIN32
86+
/**
87+
* File descriptor for the directory located at dstPath. Used for *at
88+
* operations relative to this file descriptor. This sink must *never*
89+
* follow intermediate symlinks (starting from dstPath) in case a file
90+
* collision is encountered for various reasons like case-insensitivity or
91+
* other types on normalization. using appropriate *at system calls and traversing
92+
* only one path component at a time ensures that writing is race-free and is
93+
* is not susceptible to symlink replacement.
94+
*/
95+
AutoCloseFD dirFd;
96+
#endif
8597
bool startFsync = false;
8698

8799
explicit RestoreSink(bool startFsync)

0 commit comments

Comments
 (0)