guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libstore: weaken tmpdir root access mode

Browse Source 
libarchive *should* not break with 0710 on the tmpdir root on darwin,
just like it doesn't break on linux, but for some reason it does. the
restriction to 0710 can be weakened to 0750 with causing any trouble.

fixes #921

Change-Id: Ia9fc2f8eb9695fc19cefae9857368d5a4e58c8b9
This commit is contained in:
eldritch horrors
2025-07-20 16:58:24 +00:00
parent aa9739caa7
commit fc9f7096d3
1 changed files with 7 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libstore/build/local-derivation-goal.cc
+7 -3
View File
@@ -528,8 +528,12 @@ void LocalDerivationGoal::startBuilder()
// also need the intermediate level to be inaccessible to others. build // also need the intermediate level to be inaccessible to others. build
// processes must be able to at least traverse to the directory though, // processes must be able to at least traverse to the directory though,
// without being able to chmod. this means either mode 0750 or 0710. we // without being able to chmod. this means either mode 0750 or 0710. we
// use 0710 just to be extra safe; if we ever add more directories they // cannot use 0710 because the libarchive we link with is compiled with
// will not be enumerable to other processes in the builder user group. // an old apple sdk that does not have O_SEARCH, which makes libarchive
// try to open tmpDirRoot for *read* and fail because g+r is not set. a
// future update to nixpkgs may fix this. until then we do not lose any
// security by setting mode 0750 because we use only a single subdir in
// tmpDirRoot, so being able to list its parent doesn't break anything.
// //
// use a short name to not increase the path length too much on darwin. // use a short name to not increase the path length too much on darwin.
// darwin has a severe sockaddr_un path length limitation, so this does // darwin has a severe sockaddr_un path length limitation, so this does
@@ -548,7 +552,7 @@ void LocalDerivationGoal::startBuilder()
if (fchown(tmpDirRootFd.get(), -1, buildUser->getGID()) == -1) { if (fchown(tmpDirRootFd.get(), -1, buildUser->getGID()) == -1) {
throw SysError("cannot change ownership of '%1%'", tmpDirRoot); throw SysError("cannot change ownership of '%1%'", tmpDirRoot);
} }
if (fchmod(tmpDirRootFd.get(), 0710) == -1) { if (fchmod(tmpDirRootFd.get(), 0750) == -1) {
throw SysError("cannot change mode of '%1%'", tmpDirRoot); throw SysError("cannot change mode of '%1%'", tmpDirRoot);
} }
} }