Merge remote-tracking branch 'cve/fod-cves-2.24' into 2.24-maintenance

src/libstore/local-store.cc

@@ -187,7 +187,7 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
             txn.commit();
         }
 
-        writeFile(schemaPath, fmt("%d", nixCASchemaVersion), 0666, true);
+        writeFile(schemaPath, fmt("%d", nixCASchemaVersion), 0666, FsSync::Yes);
         lockFile(lockFd.get(), ltRead, true);
     }
 }
@@ -345,7 +345,7 @@ LocalStore::LocalStore(
     else if (curSchema == 0) { /* new store */
         curSchema = nixSchemaVersion;
         openDB(*state, true);
-        writeFile(schemaPath, fmt("%1%", curSchema), 0666, true);
+        writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes);
     }
 
     else if (curSchema < nixSchemaVersion) {
@@ -394,7 +394,7 @@ LocalStore::LocalStore(
             txn.commit();
         }
 
-        writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true);
+        writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes);
 
         lockFile(globalLock.get(), ltRead, true);
     }

src/libstore/unix/build/local-derivation-goal.cc

@@ -526,7 +526,14 @@ void LocalDerivationGoal::startBuilder()
     } else {
         tmpDir = topTmpDir;
     }
-    chownToBuilder(tmpDir);
+
+    /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to
+       POSIX semantics.*/
+    tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)};
+    if (!tmpDirFd)
+        throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir);
+
+    chownToBuilder(tmpDirFd.get(), tmpDir);
 
     for (auto & [outputName, status] : initialOutputs) {
         /* Set scratch path we'll actually use during the build.
@@ -1110,9 +1117,7 @@ void LocalDerivationGoal::initTmpDir() {
             } else {
                 auto hash = hashString(HashAlgorithm::SHA256, i.first);
                 std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false);
-                Path p = tmpDir + "/" + fn;
-                writeFile(p, rewriteStrings(i.second, inputRewrites));
-                chownToBuilder(p);
+                writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites));
                 env[i.first + "Path"] = tmpDirInSandbox + "/" + fn;
             }
         }
@@ -1217,11 +1222,9 @@ void LocalDerivationGoal::writeStructuredAttrs()
 
         auto jsonSh = writeStructuredAttrsShell(json);
 
-        writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites));
-        chownToBuilder(tmpDir + "/.attrs.sh");
+        writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites));
         env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh";
-        writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites));
-        chownToBuilder(tmpDir + "/.attrs.json");
+        writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites));
         env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json";
     }
 }
@@ -1730,6 +1733,24 @@ void setupSeccomp()
 #endif
 }
 
+void LocalDerivationGoal::chownToBuilder(int fd, const Path & path)
+{
+    if (!buildUser) return;
+    if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1)
+        throw SysError("cannot change ownership of file '%1%'", path);
+}
+
+void LocalDerivationGoal::writeBuilderFile(
+    const std::string & name,
+    std::string_view contents)
+{
+    auto path = std::filesystem::path(tmpDir) / name;
+    AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)};
+    if (!fd)
+        throw SysError("creating file %s", path);
+    writeFile(fd, path, contents);
+    chownToBuilder(fd.get(), path);
+}
 
 void LocalDerivationGoal::runChild()
 {
@@ -3006,6 +3027,15 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo
 void LocalDerivationGoal::deleteTmpDir(bool force)
 {
     if (topTmpDir != "") {
+        /* As an extra precaution, even in the event of `deletePath` failing to
+         * clean up, the `tmpDir` will be chowned as if we were to move
+         * it inside the Nix store.
+         *
+         * This hardens against an attack which smuggles a file descriptor
+         * to make use of the temporary directory.
+         */
+        chmod(topTmpDir.c_str(), 0000);
+
         /* Don't keep temporary directories for builtins because they
            might have privileged stuff (like a copy of netrc). */
         if (settings.keepFailed && !force && !drv->isBuiltin()) {

src/libstore/unix/build/local-derivation-goal.hh

@@ -37,6 +37,11 @@ struct LocalDerivationGoal : public DerivationGoal
      */
     Path topTmpDir;
 
+    /**
+     * The file descriptor of the temporary directory.
+     */
+    AutoCloseFD tmpDirFd;
+
     /**
      * The path of the temporary directory in the sandbox.
      */
@@ -232,9 +237,24 @@ struct LocalDerivationGoal : public DerivationGoal
 
     /**
      * Make a file owned by the builder.
+     *
+     * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor.
+     * It's only safe to call in a child of a directory only visible to the owner.
      */
     void chownToBuilder(const Path & path);
 
+    /**
+     * Make a file owned by the builder addressed by its file descriptor.
+     */
+    void chownToBuilder(int fd, const Path & path);
+
+    /**
+     * Create a file in `tmpDir` owned by the builder.
+     */
+    void writeBuilderFile(
+        const std::string & name,
+        std::string_view contents);
+
     int getChildStatus() override;
 
     /**

src/libutil/file-system.cc

@@ -247,7 +247,7 @@ void readFile(const Path & path, Sink & sink)
 }
 
 
-void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync)
+void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync)
 {
     AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT
 // TODO
@@ -257,22 +257,29 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync)
        , mode));
     if (!fd)
         throw SysError("opening file '%1%'", path);
-    try {
-        writeFull(fd.get(), s);
-    } catch (Error & e) {
-        e.addTrace({}, "writing file '%1%'", path);
-        throw;
-    }
-    if (sync)
-        fd.fsync();
-    // Explicitly close to make sure exceptions are propagated.
+
+    writeFile(fd, path, s, mode, sync);
+
+    /* Close explicitly to propagate the exceptions. */
     fd.close();
-    if (sync)
-        syncParent(path);
 }
 
+void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync)
+{
+    assert(fd);
+    try {
+        writeFull(fd.get(), s);
 
-void writeFile(const Path & path, Source & source, mode_t mode, bool sync)
+        if (sync == FsSync::Yes)
+            fd.fsync();
+
+    } catch (Error & e) {
+        e.addTrace({}, "writing file '%1%'", origPath);
+        throw;
+    }
+}
+
+void writeFile(const Path & path, Source & source, mode_t mode, FsSync sync)
 {
     AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT
 // TODO
@@ -296,11 +303,11 @@ void writeFile(const Path & path, Source & source, mode_t mode, bool sync)
         e.addTrace({}, "writing file '%1%'", path);
         throw;
     }
-    if (sync)
+    if (sync == FsSync::Yes)
         fd.fsync();
     // Explicitly close to make sure exceptions are propagated.
     fd.close();
-    if (sync)
+    if (sync == FsSync::Yes)
         syncParent(path);
 }
 
@@ -318,7 +325,8 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b
 #ifndef _WIN32
     checkInterrupt();
 
-    std::string name(baseNameOf(path.native()));
+    std::string name(path.filename());
+    assert(name != "." && name != ".." && !name.empty());
 
     struct stat st;
     if (fstatat(parentfd, name.c_str(), &st,
@@ -359,7 +367,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b
                 throw SysError("chmod '%1%'", path);
         }
 
-        int fd = openat(parentfd, path.c_str(), O_RDONLY);
+        int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
         if (fd == -1)
             throw SysError("opening directory '%1%'", path);
         AutoCloseDir dir(fdopendir(fd));
@@ -371,7 +379,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b
             checkInterrupt();
             std::string childName = dirent->d_name;
             if (childName == "." || childName == "..") continue;
-            _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed);
+            _deletePath(dirfd(dir.get()), path / childName, bytesFreed);
         }
         if (errno) throw SysError("reading directory '%1%'", path);
     }
@@ -389,14 +397,13 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b
 
 static void _deletePath(const fs::path & path, uint64_t & bytesFreed)
 {
-    Path dir = dirOf(path.string());
-    if (dir == "")
-        dir = "/";
+    assert(path.is_absolute());
+    assert(path.parent_path() != path);
 
-    AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY));
+    AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY));
     if (!dirfd) {
         if (errno == ENOENT) return;
-        throw SysError("opening directory '%1%'", path);
+        throw SysError("opening directory %s", path.parent_path());
     }
 
     _deletePath(dirfd.get(), path, bytesFreed);

src/libutil/file-system.hh

@@ -148,12 +148,16 @@ Descriptor openDirectory(const std::filesystem::path & path);
 std::string readFile(const Path & path);
 void readFile(const Path & path, Sink & sink);
 
+enum struct FsSync { Yes, No };
+
 /**
  * Write a string to a file.
  */
-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false);
+void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No);
 
-void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false);
+void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No);
+
+void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No);
 
 /**
  * Flush a file's parent directory to disk