CMXSL.org/package-helpers-cmxsl
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
package-helpers-cmxsl/helpers/DATA/guix/cve/CVE-2024-27297_4a67c00ad02fbe7a7f5796c4c4dc2c0ad70f0472.patch

378 lines
15 KiB
Diff
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

From 4a67c00ad02fbe7a7f5796c4c4dc2c0ad70f0472 Mon Sep 17 00:00:00 2001
From: Vagrant Cascadian <vagrant@debian.org>
Date: Tue, 12 Mar 2024 09:18:23 -0700
Subject: [PATCH] debian/patches: guix-daemon: Protect against file descriptor
escape when building fixed-output derivations (CVE-2024-27297). (Closes:
#1066113)
---
...gainst-FD-escape-when-building-fixed.patch | 232 ++++++++++++++++++
...hortcoming-in-previous-security-fix-.patch | 106 ++++++++
debian/patches/series | 2 +
3 files changed, 340 insertions(+)
create mode 100644 debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch
create mode 100644 debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch
diff --git a/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch b/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch
new file mode 100644
index 0000000000..e6e02cf206
--- /dev/null
+++ b/debian/patches/security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch
@@ -0,0 +1,232 @@
+From 8f4ffb3fae133bb21d7991e97c2f19a7108b1143 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
+Date: Mon, 11 Mar 2024 10:59:42 +0100
+Subject: [PATCH 01/36] daemon: Protect against FD escape when building
+ fixed-output derivations (CVE-2024-27297).
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+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”.
+
+Vulnerability discovered by puck <https://github.com/puckipedia>.
+
+Nix security advisory:
+https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37
+
+Nix fix:
+https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9
+
+* 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(-)
+
+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);
+--
+2.39.2
+
diff --git a/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch b/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch
new file mode 100644
index 0000000000..0d0b6bd22f
--- /dev/null
+++ b/debian/patches/security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch
@@ -0,0 +1,106 @@
+From ff1251de0bc327ec478fc66a562430fbf35aef42 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
+Date: Tue, 12 Mar 2024 11:53:35 +0100
+Subject: [PATCH 32/36] daemon: Address shortcoming in previous security fix
+ for CVE-2024-27297.
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+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
+---
+ nix/libstore/build.cc | 11 ++++++-----
+ nix/libutil/util.cc | 4 ++++
+ nix/libutil/util.hh | 7 ++++---
+ 3 files changed, 14 insertions(+), 8 deletions(-)
+
+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);
+
+--
+2.39.2
+
diff --git a/debian/patches/series b/debian/patches/series_
index 5d506e57..0b8879d1 100644
--- a/debian/patches/series
+++ b/debian/patches/series_
@@ -40,3 +40,5 @@ lsb-init-functions
guix-daemon-openrc-fixes
tests-Ensure-test-OpenPGP-keys-never-expire.patch
use-c-utf8-locale
+security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch
+security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch
--
GitLab
Reference in a new issue View git blame Copy permalink