Version in base suite: 1.4.0-3+deb12u1 Base version: guix_1.4.0-3+deb12u1 Target version: guix_1.4.0-3+deb12u2 Base file: /srv/ftp-master.debian.org/ftp/pool/main/g/guix/guix_1.4.0-3+deb12u1.dsc Target file: /srv/ftp-master.debian.org/policy/pool/main/g/guix/guix_1.4.0-3+deb12u2.dsc changelog | 6 patches/security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch | 85 ++++++++++ patches/security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch | 65 +++++++ patches/series | 3 4 files changed, 159 insertions(+) diff -Nru guix-1.4.0/debian/changelog guix-1.4.0/debian/changelog --- guix-1.4.0/debian/changelog 2024-04-17 21:23:27.000000000 +0000 +++ guix-1.4.0/debian/changelog 2024-11-06 00:22:53.000000000 +0000 @@ -1,3 +1,9 @@ +guix (1.4.0-3+deb12u2) bookworm-security; urgency=medium + + * debian/patches: Add patches to fix Build User Takeover Vulnerability. + + -- Vagrant Cascadian Tue, 05 Nov 2024 16:22:53 -0800 + guix (1.4.0-3+deb12u1) bookworm-security; urgency=medium * debian/patches: guix-daemon: Protect against file descriptor escape diff -Nru guix-1.4.0/debian/patches/security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch guix-1.4.0/debian/patches/security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch --- guix-1.4.0/debian/patches/security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch 1970-01-01 00:00:00.000000000 +0000 +++ guix-1.4.0/debian/patches/security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch 2024-11-06 00:20:09.000000000 +0000 @@ -0,0 +1,85 @@ +From 558224140dab669cabdaebabff18504a066c48d4 Mon Sep 17 00:00:00 2001 +From: Reepca Russelstein +Date: Sun, 20 Oct 2024 15:36:06 -0500 +Subject: [PATCH 1/2] daemon: Sanitize failed build outputs prior to exposing + them. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The only thing keeping a rogue builder and a local user from collaborating to +usurp control over the builder's user during the build is the fact that +whatever files the builder may produce are not accessible to any other users +yet. If we're going to make them accessible, we should probably do some +sanity checking to ensure that sort of collaborating can't happen. + +Currently this isn't happening when failed build outputs are moved from the +chroot as an aid to debugging. + +* nix/libstore/build.cc (secureFilePerms): new function. + (DerivationGoal::buildDone): use it. + +Change-Id: I9dce1e3d8813b31cabd87a0e3219bf9830d8be96 +Signed-off-by: Ludovic Courtès +--- + nix/libstore/build.cc | 36 +++++++++++++++++++++++++++++++++++- + 1 file changed, 35 insertions(+), 1 deletion(-) + +diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc +index d23c0944a4..67ebfe2f14 100644 +--- a/nix/libstore/build.cc ++++ b/nix/libstore/build.cc +@@ -1301,6 +1301,34 @@ void replaceValidPath(const Path & storePath, const Path tmpPath) + MakeError(NotDeterministic, BuildError) + + ++/* Recursively make the file permissions of a path safe for exposure to ++ arbitrary users, but without canonicalising its permissions, timestamp, and ++ user. Throw an exception if a file type that isn't explicitly known to be ++ safe is found. */ ++static void secureFilePerms(Path path) ++{ ++ struct stat st; ++ if (lstat(path.c_str(), &st)) return; ++ ++ switch(st.st_mode & S_IFMT) { ++ case S_IFLNK: ++ return; ++ ++ case S_IFDIR: ++ for (auto & i : readDirectory(path)) { ++ secureFilePerms(path + "/" + i.name); ++ } ++ /* FALLTHROUGH */ ++ ++ case S_IFREG: ++ chmod(path.c_str(), (st.st_mode & ~S_IFMT) & ~(S_ISUID | S_ISGID | S_IWOTH)); ++ break; ++ ++ default: ++ throw Error(format("file `%1%' has an unsupported type") % path); ++ } ++} ++ + void DerivationGoal::buildDone() + { + trace("build done"); +@@ -1372,8 +1400,14 @@ void DerivationGoal::buildDone() + build failures. */ + if (useChroot && buildMode == bmNormal) + foreach (PathSet::iterator, i, missingPaths) +- if (pathExists(chrootRootDir + *i)) ++ if (pathExists(chrootRootDir + *i)) { ++ try { ++ secureFilePerms(chrootRootDir + *i); + rename((chrootRootDir + *i).c_str(), i->c_str()); ++ } catch(Error & e) { ++ printMsg(lvlError, e.msg()); ++ } ++ } + + if (diskFull) + printMsg(lvlError, "note: build failure may have been caused by lack of free disk space"); +-- +2.39.5 + diff -Nru guix-1.4.0/debian/patches/security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch guix-1.4.0/debian/patches/security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch --- guix-1.4.0/debian/patches/security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch 1970-01-01 00:00:00.000000000 +0000 +++ guix-1.4.0/debian/patches/security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch 2024-11-06 00:20:09.000000000 +0000 @@ -0,0 +1,65 @@ +From 5ab3c4c1e43ebb637551223791db0ea3519986e1 Mon Sep 17 00:00:00 2001 +From: Reepca Russelstein +Date: Sun, 20 Oct 2024 15:39:02 -0500 +Subject: [PATCH 2/2] daemon: Sanitize successful build outputs prior to + exposing them. +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +There is currently a window of time between when the build outputs are exposed +and when their metadata is canonicalized. + +* nix/libstore/build.cc (DerivationGoal::registerOutputs): wait until after + metadata canonicalization to move successful build outputs to the store. + +Change-Id: Ia995136f3f965eaf7b0e1d92af964b816f3fb276 +Signed-off-by: Ludovic Courtès +--- + nix/libstore/build.cc | 23 ++++++++++++++--------- + 1 file changed, 14 insertions(+), 9 deletions(-) + +diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc +index 67ebfe2f14..43a8a37184 100644 +--- a/nix/libstore/build.cc ++++ b/nix/libstore/build.cc +@@ -2369,15 +2369,6 @@ void DerivationGoal::registerOutputs() + Path actualPath = path; + if (useChroot) { + actualPath = chrootRootDir + path; +- if (pathExists(actualPath)) { +- /* Move output paths from the chroot to the store. */ +- if (buildMode == bmRepair) +- replaceValidPath(path, actualPath); +- else +- if (buildMode != bmCheck && rename(actualPath.c_str(), path.c_str()) == -1) +- throw SysError(format("moving build output `%1%' from the chroot to the store") % path); +- } +- if (buildMode != bmCheck) actualPath = path; + } else { + Path redirected = redirectedOutputs[path]; + if (buildMode == bmRepair +@@ -2463,6 +2454,20 @@ void DerivationGoal::registerOutputs() + canonicalisePathMetaData(actualPath, + buildUser.enabled() && !rewritten ? buildUser.getUID() : -1, inodesSeen); + ++ if (useChroot) { ++ if (pathExists(actualPath)) { ++ /* Now that output paths have been canonicalized (in particular ++ there are no setuid files left), move them outside of the ++ chroot and to the store. */ ++ if (buildMode == bmRepair) ++ replaceValidPath(path, actualPath); ++ else ++ if (buildMode != bmCheck && rename(actualPath.c_str(), path.c_str()) == -1) ++ throw SysError(format("moving build output `%1%' from the chroot to the store") % path); ++ } ++ if (buildMode != bmCheck) actualPath = path; ++ } ++ + /* For this output path, find the references to other paths + contained in it. Compute the SHA-256 NAR hash at the same + time. The hash is stored in the database so that we can +-- +2.39.5 + diff -Nru guix-1.4.0/debian/patches/series guix-1.4.0/debian/patches/series --- guix-1.4.0/debian/patches/series 2024-04-17 21:22:05.000000000 +0000 +++ guix-1.4.0/debian/patches/series 2024-11-06 00:20:09.000000000 +0000 @@ -50,3 +50,6 @@ tests-profiles.scm-Disable-profile-derivation-format.patch security/0001-daemon-Protect-against-FD-escape-when-building-fixed.patch security/0032-daemon-Address-shortcoming-in-previous-security-fix-.patch +# https://guix.gnu.org/en/blog/2024/build-user-takeover-vulnerability/ +security/0101-daemon-Sanitize-failed-build-outputs-prior-to-exposi.patch +security/0102-daemon-Sanitize-successful-build-outputs-prior-to-ex.patch