mirror of
https://github.com/veracrypt/VeraCrypt.git
synced 2026-06-18 02:26:07 -05:00
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 <damian@rickard.us>
This commit is contained in:
@@ -12,6 +12,7 @@
|
|||||||
|
|
||||||
#include "CoreService.h"
|
#include "CoreService.h"
|
||||||
#include <fcntl.h>
|
#include <fcntl.h>
|
||||||
|
#include <sys/stat.h>
|
||||||
#include <sys/wait.h>
|
#include <sys/wait.h>
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include "Platform/FileStream.h"
|
#include "Platform/FileStream.h"
|
||||||
@@ -60,6 +61,27 @@ namespace VeraCrypt
|
|||||||
|| IsMacOSXDevicePathWithPrefix (path, "/dev/rdisk");
|
|| 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 <string> BuildMacOSXAPFSFormatterArguments (const ExecuteMacOSXAPFSFormatterRequest &request)
|
static list <string> BuildMacOSXAPFSFormatterArguments (const ExecuteMacOSXAPFSFormatterRequest &request)
|
||||||
{
|
{
|
||||||
if (!IsMacOSXFormatterDevicePath (request.Device))
|
if (!IsMacOSXFormatterDevicePath (request.Device))
|
||||||
@@ -292,6 +314,10 @@ namespace VeraCrypt
|
|||||||
if (!coreUnix)
|
if (!coreUnix)
|
||||||
throw ParameterIncorrect (SRC_POS);
|
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);
|
coreUnix->SetFileOwner (setFileOwnerRequest->Path, setFileOwnerRequest->Owner);
|
||||||
SetFileOwnerResponse().Serialize (outputStream);
|
SetFileOwnerResponse().Serialize (outputStream);
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
Reference in New Issue
Block a user