Skip to content
Snippets Groups Projects
Commit dbda85fd authored by Luis Guzmán's avatar Luis Guzmán
Browse files

guix: add patches to fix guix#73919.

parent 1e8d358c
No related branches found
No related tags found
1 merge request!1541guix: add patches to fix guix#73919.
From 532996c5908fb14cc8d102865280fb203c075c9c Mon Sep 17 00:00:00 2001
From: Reepca Russelstein <reepca@russelstein.xyz>
Date: Sun, 20 Oct 2024 17:32:23 -0500
Subject: [PATCH] etc: news: add news entry for build user takeover
vulnerability fix.
* etc/news.scm: add entry about build user takeover vulnerability.
---
etc/news.scm | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/etc/news.scm b/etc/news.scm
index a90f92a9ff..3fb53a9849 100644
--- a/etc/news.scm
+++ b/etc/news.scm
@@ -33,6 +33,38 @@
(channel-news
(version 0)
+ (entry (commit "5966e0fdc78771c562e0f484a22f381a77908be0")
+ (title
+ (en "Daemon vulnerability allowing takeover of build users fixed"))
+ (body
+ (en "A vulnerability allowing a local user to execute arbitrary code
+as any of the build users has been identified and fixed. Most notably, this
+allows any local user to alter the result of any local build, even if it
+happens inside a container. The only requirements to exploit this
+vulnerability are the ability to start a derivation build and the ability to
+run arbitrary code with access to the store in the root PID namespace on the
+machine that build occurs on. This largely limits the vulnerability to
+multi-user systems.
+
+This vulnerability is caused by the fact that @command{guix-daemon} does not
+change ownership and permissions on the outputs of failed builds when it moves
+them to the store, and is also caused by there being a window of time between
+when it moves outputs of successful builds to the store and when it changes
+their ownership and permissions. Because of this, a build can create a binary
+with both setuid and setgid bits set and have it become visible to the outside
+world once the build ends. At that point any process that can access the
+store can execute it and gain the build user's privileges. From there any
+process owned by that build user can be manipulated via procfs and signals at
+will, allowing the attacker to control the output of its builds.
+
+You are advised to upgrade @command{guix-daemon}. Run @command{info \"(guix)
+Upgrading Guix\"}, for info on how to do that. Additionally, if there is any
+risk that a builder may have already created these setuid binaries (for
+example on accident), run @command{guix gc} to remove all failed build
+outputs.
+
+See @uref{https://issues.guix.gnu.org/73919} for more information on this
+vulnerability.")))
(entry (commit "2161820ebbbab62a5ce76c9101ebaec54dc61586")
(title
(en "Risk of local privilege escalation during user account creation")
--
2.45.2
From e936861263d9bafdfbe395c12526f2dc48ac17d7 Mon Sep 17 00:00:00 2001
Message-ID: <e936861263d9bafdfbe395c12526f2dc48ac17d7.1729457080.git.reepca@russelstein.xyz>
From: Reepca Russelstein <reepca@russelstein.xyz>
Date: Sun, 20 Oct 2024 15:36:06 -0500
Subject: [PATCH 1/2] nix: build: sanitize failed build outputs prior to
exposing them.
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
---
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,9 +1400,15 @@ 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.45.2
From d096d653cc69118e05f49247ab312d0096b16656 Mon Sep 17 00:00:00 2001
Message-ID: <d096d653cc69118e05f49247ab312d0096b16656.1729457080.git.reepca@russelstein.xyz>
In-Reply-To: <e936861263d9bafdfbe395c12526f2dc48ac17d7.1729457080.git.reepca@russelstein.xyz>
References: <e936861263d9bafdfbe395c12526f2dc48ac17d7.1729457080.git.reepca@russelstein.xyz>
From: Reepca Russelstein <reepca@russelstein.xyz>
Date: Sun, 20 Oct 2024 15:39:02 -0500
Subject: [PATCH 2/2] nix: build: sanitize successful build outputs prior to
exposing them.
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
---
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.45.2
......@@ -17,7 +17,8 @@
# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
#
VERSION=0
VERSION=1
SECURITY=true
. ./config
export FULLVERSION="$(sed 's|1.3.0-4|1.3.0-4+really1.3.0-5|' <<< $FULLVERSION)"
......@@ -27,6 +28,7 @@ patch_p1 $DATA/guix-1.3.0.4-to-1.3.0-5.patch
# Apply missing CVEs upstream:
## 1.CVE-2024-27297
## 2. https://issues.guix.gnu.org/73919
for patch in $(ls -v ${DATA}/cve/*.patch)
do
patch_p1 $patch
......@@ -35,6 +37,6 @@ done
# TODO: Confirm fix for nix package.
# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1066812
changelog "Upgrade to 1.3.0-5 and apply missing CVE-2024-27297 upstream patch."
changelog "Apply latest fix for #73919 || Upgrade to 1.3.0-5 and apply missing CVE-2024-27297 upstream patch."
package
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment