@@ -129,6 +129,11 @@ class DerivationBuilderImpl : public DerivationBuilder, DerivationBuilderParams
129129 */
130130 Path topTmpDir;
131131
132+ /* *
133+ * The file descriptor of the temporary directory.
134+ */
135+ AutoCloseFD tmpDirFd;
136+
132137 /* *
133138 * The path of the temporary directory in the sandbox.
134139 */
@@ -325,9 +330,24 @@ class DerivationBuilderImpl : public DerivationBuilder, DerivationBuilderParams
325330
326331 /* *
327332 * Make a file owned by the builder.
333+ *
334+ * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor.
335+ * It's only safe to call in a child of a directory only visible to the owner.
328336 */
329337 void chownToBuilder (const Path & path);
330338
339+ /* *
340+ * Make a file owned by the builder addressed by its file descriptor.
341+ */
342+ void chownToBuilder (int fd, const Path & path);
343+
344+ /* *
345+ * Create a file in `tmpDir` owned by the builder.
346+ */
347+ void writeBuilderFile (
348+ const std::string & name,
349+ std::string_view contents);
350+
331351 /* *
332352 * Run the builder's process.
333353 */
@@ -900,7 +920,14 @@ void DerivationBuilderImpl::startBuilder()
900920 } else {
901921 tmpDir = topTmpDir;
902922 }
903- chownToBuilder (tmpDir);
923+
924+ /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to
925+ POSIX semantics.*/
926+ tmpDirFd = AutoCloseFD{open (tmpDir.c_str (), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)};
927+ if (!tmpDirFd)
928+ throw SysError (" failed to open the build temporary directory descriptor '%1%'" , tmpDir);
929+
930+ chownToBuilder (tmpDirFd.get (), tmpDir);
904931
905932 for (auto & [outputName, status] : initialOutputs) {
906933 /* Set scratch path we'll actually use during the build.
@@ -1485,9 +1512,7 @@ void DerivationBuilderImpl::initTmpDir()
14851512 } else {
14861513 auto hash = hashString (HashAlgorithm::SHA256, i.first );
14871514 std::string fn = " .attr-" + hash.to_string (HashFormat::Nix32, false );
1488- Path p = tmpDir + " /" + fn;
1489- writeFile (p, rewriteStrings (i.second , inputRewrites));
1490- chownToBuilder (p);
1515+ writeBuilderFile (fn, rewriteStrings (i.second , inputRewrites));
14911516 env[i.first + " Path" ] = tmpDirInSandbox + " /" + fn;
14921517 }
14931518 }
@@ -1596,11 +1621,9 @@ void DerivationBuilderImpl::writeStructuredAttrs()
15961621
15971622 auto jsonSh = StructuredAttrs::writeShell (json);
15981623
1599- writeFile (tmpDir + " /.attrs.sh" , rewriteStrings (jsonSh, inputRewrites));
1600- chownToBuilder (tmpDir + " /.attrs.sh" );
1624+ writeBuilderFile (" .attrs.sh" , rewriteStrings (jsonSh, inputRewrites));
16011625 env[" NIX_ATTRS_SH_FILE" ] = tmpDirInSandbox + " /.attrs.sh" ;
1602- writeFile (tmpDir + " /.attrs.json" , rewriteStrings (json.dump (), inputRewrites));
1603- chownToBuilder (tmpDir + " /.attrs.json" );
1626+ writeBuilderFile (" .attrs.json" , rewriteStrings (json.dump (), inputRewrites));
16041627 env[" NIX_ATTRS_JSON_FILE" ] = tmpDirInSandbox + " /.attrs.json" ;
16051628 }
16061629}
@@ -1854,6 +1877,24 @@ void setupSeccomp()
18541877#endif
18551878}
18561879
1880+ void DerivationBuilderImpl::chownToBuilder (int fd, const Path & path)
1881+ {
1882+ if (!buildUser) return ;
1883+ if (fchown (fd, buildUser->getUID (), buildUser->getGID ()) == -1 )
1884+ throw SysError (" cannot change ownership of file '%1%'" , path);
1885+ }
1886+
1887+ void DerivationBuilderImpl::writeBuilderFile (
1888+ const std::string & name,
1889+ std::string_view contents)
1890+ {
1891+ auto path = std::filesystem::path (tmpDir) / name;
1892+ AutoCloseFD fd{openat (tmpDirFd.get (), name.c_str (), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666 )};
1893+ if (!fd)
1894+ throw SysError (" creating file %s" , path);
1895+ writeFile (fd, path, contents);
1896+ chownToBuilder (fd.get (), path);
1897+ }
18571898
18581899void DerivationBuilderImpl::runChild ()
18591900{
@@ -3065,6 +3106,15 @@ void DerivationBuilderImpl::checkOutputs(const std::map<std::string, ValidPathIn
30653106void DerivationBuilderImpl::deleteTmpDir (bool force)
30663107{
30673108 if (topTmpDir != " " ) {
3109+ /* As an extra precaution, even in the event of `deletePath` failing to
3110+ * clean up, the `tmpDir` will be chowned as if we were to move
3111+ * it inside the Nix store.
3112+ *
3113+ * This hardens against an attack which smuggles a file descriptor
3114+ * to make use of the temporary directory.
3115+ */
3116+ chmod (topTmpDir.c_str (), 0000 );
3117+
30683118 /* Don't keep temporary directories for builtins because they
30693119 might have privileged stuff (like a copy of netrc). */
30703120 if (settings.keepFailed && !force && !drv.isBuiltin ()) {
0 commit comments