From 575262a104a15bb3990e7256aa81e1123715b60a Mon Sep 17 00:00:00 2001 From: damianrickard <154095072+damianrickard@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:31:42 -0400 Subject: [PATCH] macOS: restrict elevated SetFileOwner to disk device nodes (#1758) The privileged CoreService handler for SetFileOwnerRequest passed the client-supplied path straight to chown() as root with no validation -- unlike the adjacent APFS formatter handler, which strictly validates its device argument. Every legitimate macOS caller of the elevated SetFileOwner targets a real disk device node (/dev/[r]diskN[sM]), so a crafted IPC request, or a symlink planted at the target, could otherwise make the root process change ownership of an arbitrary path. Validate the target service-side: require the strict device-path form already used by the formatter, and lstat() it to confirm a block or character device (rejecting symlinks rather than following them) before the chown. Co-authored-by: Damian Rickard --- src/Core/Unix/CoreService.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/Core/Unix/CoreService.cpp b/src/Core/Unix/CoreService.cpp index 14426f4d..e69df959 100644 --- a/src/Core/Unix/CoreService.cpp +++ b/src/Core/Unix/CoreService.cpp @@ -12,6 +12,7 @@ #include "CoreService.h" #include +#include #include #include #include "Platform/FileStream.h" @@ -60,6 +61,27 @@ namespace VeraCrypt || IsMacOSXDevicePathWithPrefix (path, "/dev/rdisk"); } + // The elevated service runs as root, so it must not be tricked into changing + // ownership of an arbitrary path. Every legitimate macOS caller of the + // elevated SetFileOwner targets a real disk device node (/dev/[r]diskN[sM]), + // so restrict the operation to that. lstat() (not stat) is used so a symlink + // is rejected outright rather than followed, and the st_mode check confirms an + // actual block/character device before the chown. + static void ValidateMacOSXSetFileOwnerTarget (const FilesystemPath &path) + { + const string pathStr = path; + + if (!IsMacOSXFormatterDevicePath (pathStr)) + throw ParameterIncorrect (SRC_POS); + + struct stat sb; + if (lstat (pathStr.c_str(), &sb) != 0) + throw ParameterIncorrect (SRC_POS); + + if (!S_ISBLK (sb.st_mode) && !S_ISCHR (sb.st_mode)) + throw ParameterIncorrect (SRC_POS); + } + static list BuildMacOSXAPFSFormatterArguments (const ExecuteMacOSXAPFSFormatterRequest &request) { if (!IsMacOSXFormatterDevicePath (request.Device)) @@ -292,6 +314,10 @@ namespace VeraCrypt if (!coreUnix) throw ParameterIncorrect (SRC_POS); +#ifdef TC_MACOSX + // Restrict the root-privileged chown to real disk device nodes. + ValidateMacOSXSetFileOwnerTarget (setFileOwnerRequest->Path); +#endif coreUnix->SetFileOwner (setFileOwnerRequest->Path, setFileOwnerRequest->Owner); SetFileOwnerResponse().Serialize (outputStream); continue;