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

1 files changed, 6 insertions, 5 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);
 		}
 	    }
 	}