[PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297).

  • Done
  • quality assurance status badge
Details
2 participants
  • John Kehayias
  • Ludovic Courtès
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
normal

Debbugs page

L
L
Ludovic Courtès wrote on 11 Mar 03:54 -0700
(address . guix-patches@gnu.org)
f541e64f128d82e6d9eca3b1d40e833dc06fd968.1710154382.git.ludo@gnu.org
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:

* 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:


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’.

Toggle diff (193 lines)
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
L
L
Ludovic Courtès wrote on 11 Mar 08:03 -0700
control message for bug #69728
(address . control@debbugs.gnu.org)
87edcgq08e.fsf@gnu.org
tags 69728 + security
quit
L
L
Ludovic Courtès wrote on 11 Mar 15:16 -0700
Re: bug#69728: [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297).
(address . 69728@debbugs.gnu.org)
87frwwo1mo.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (26 lines)
> 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’.
J
J
John Kehayias wrote on 11 Mar 17:42 -0700
(name . Ludovic Courtès)(address . ludo@gnu.org)
87o7bk2sc6.fsf@protonmail.com
Hi all,

On Mon, Mar 11, 2024 at 11:16 PM, Ludovic Courtès wrote:

Toggle quote (39 lines)
> 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
L
L
Ludovic Courtès wrote on 12 Mar 06:31 -0700
(address . 69728@debbugs.gnu.org)
87ttlblgq3.fsf_-_@gnu.org
Hello,

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (8 lines)
> 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’.
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

Toggle diff (70 lines)
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);
L
L
Ludovic Courtès wrote on 12 Mar 06:45 -0700
Reproducer for the daemon fixed-output derivation vulnerability
(address . 69728@debbugs.gnu.org)
871q8flg17.fsf_-_@gnu.org
As promised, attached is a reproducer that I adapted from the Nix one at
https://hackmd.io/03UGerewRcy3db44JQoWvw, which I think was written by

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:

Toggle snippet (31 lines)
$ 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

On a system that is still vulnerable, we get this instead:

Toggle snippet (34 lines)
$ 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

At this point,
/gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261
is corrupt:

Toggle snippet (4 lines)
$ cat /gnu/store/yls7xkg8k0i0qxab8sv960qsy6a0xcz7-derivation-that-exfiltrates-fd-65f05aca-17261
This file has been corrupted!

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’.
-----BEGIN PGP SIGNATURE-----

iQJBBAEBCgArFiEEPORkVYqE/cadtAz7CQsRmT2a67UFAmXwXJQNHGx1ZG9AZ251
Lm9yZwAKCRAJCxGZPZrrtZQED/9cu9Wq+0e6bKFMCXjpF5DSjBbuZ/jQTBtyC2NK
wOCNzeY2Us3C9gGp9y+78Bq8JIOAmZK/zUFuFotR4e9mcaD2SiKQYkQq913xNsvu
8qS/si39CmPRltbbT6ExxIulrLfRgaGXQudWCEn457VCX04WlvyMhVUTpMzEqaYF
jml1UKKxozrkn9OGqiaLy5L5E8HQuTr8ZUOxHfi0omT0yP+KOxfZTuEoYB+hAPSS
oIa97fsredd38XjJ6SzK2O4wurUVzwFR8DzIMGUkJj9StJboizEQsDtL1JjRoNv4
g8cxatt5m0Dr5pb7FDamoUHSB/GcOOoTmhg7WXOn3anNB58nCWGJynKZqzSMJ0W4
r+Mu3MLnV4E4ugOiBbqT9rfVQ/O7+YlrUJpJMzZetsLestetsGCIuk7NMDbUC7QW
jjXE3CFQagqSVDcsURvjLyRUztgRHvsN9mfAjeG03mOCoadSvlz8zCGQyBWavSJ2
mHNkW2H7WioklHYqPh7J0XCGCd7uwKD7vUqPkqso2ogVvzheD/mliAq9Z7TaP+2u
YJbTJXFxYA7ZmiOk2hYBGRzF2kEYICX7jZUyE9pmFG/Dqm1yLMdpgLFeZOTywrto
Tz927oa6UGyLYMgly4BRxGKfsgQni38vQlUqYY1pch0eSv4d+1vF6lzodt9sigZD
m4EaMg==
=h0hp
-----END PGP SIGNATURE-----

J
J
John Kehayias wrote on 12 Mar 07:35 -0700
(name . Ludovic Courtès)(address . ludo@gnu.org)
87msr334do.fsf@protonmail.com
-----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:

Toggle quote (8 lines)
> 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
L
L
Ludovic Courtès wrote on 12 Mar 08:31 -0700
Re: bug#69728: [PATCH security] daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297).
(name . John Kehayias)(address . john.kehayias@protonmail.com)
87a5n3jwln.fsf_-_@gnu.org
Hi John,

John Kehayias <john.kehayias@protonmail.com> skribis:

Toggle quote (6 lines)
> 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’.
L
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 69728@patchwise.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 69728
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch