Skip to content

Commit ae1bff0

Browse files
janvorlijanvorli
authored
Restrict named mutex files permissions (#78106)
This change restricts the permission for files created for named mutexes that are not machine wide. Until now, all named mutexes had underlying files with access to all users. With this change, mutexes that are session local have access for the current user only. In addition to that, sticky bit is set for the /tmp/.dotnet, /tmp/.dotnet/shm directories to ensure that only creator of a file or a subdirectory can delete the respective file / subdirectory.
1 parent c04cf0c commit ae1bff0

3 files changed

Lines changed: 42 additions & 18 deletions

File tree

‎src/coreclr/pal/src/include/pal/sharedmemory.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,11 @@ class SharedMemoryException
8888
class SharedMemoryHelpers
8989
{
9090
private:
91+
static const mode_t PermissionsMask_CurrentUser_ReadWrite;
9192
static const mode_t PermissionsMask_CurrentUser_ReadWriteExecute;
9293
static const mode_t PermissionsMask_AllUsers_ReadWrite;
9394
static const mode_t PermissionsMask_AllUsers_ReadWriteExecute;
95+
9496
public:
9597
static const UINT32 InvalidProcessId;
9698
static const SIZE_T InvalidThreadId;
@@ -106,12 +108,12 @@ class SharedMemoryHelpers
106108
static void BuildSharedFilesPath(PathCharString& destination, const char *suffix, int suffixByteCount);
107109
static bool AppendUInt32String(PathCharString& destination, UINT32 value);
108110

109-
static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool createIfNotExist = true, bool isSystemDirectory = false);
111+
static bool EnsureDirectoryExists(const char *path, bool isGlobalLockAcquired, bool hasCurrentUserAccessOnly, bool setStickyFlag, bool createIfNotExist = true, bool isSystemDirectory = false);
110112
private:
111113
static int Open(LPCSTR path, int flags, mode_t mode = static_cast<mode_t>(0));
112114
public:
113115
static int OpenDirectory(LPCSTR path);
114-
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool *createdRef = nullptr);
116+
static int CreateOrOpenFile(LPCSTR path, bool createIfNotExist = true, bool isSessionScope = true, bool *createdRef = nullptr);
115117
static void CloseFile(int fileDescriptor);
116118

117119
static SIZE_T GetFileSize(int fileDescriptor);

‎src/coreclr/pal/src/sharedmemory/sharedmemory.cpp‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ DWORD SharedMemoryException::GetErrorCode() const
6262
////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
6363
// SharedMemoryHelpers
6464

65+
const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWrite = S_IRUSR | S_IWUSR;
6566
const mode_t SharedMemoryHelpers::PermissionsMask_CurrentUser_ReadWriteExecute = S_IRUSR | S_IWUSR | S_IXUSR;
6667
const mode_t SharedMemoryHelpers::PermissionsMask_AllUsers_ReadWrite =
6768
S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
@@ -96,13 +97,22 @@ SIZE_T SharedMemoryHelpers::AlignUp(SIZE_T value, SIZE_T alignment)
9697
bool SharedMemoryHelpers::EnsureDirectoryExists(
9798
const char *path,
9899
bool isGlobalLockAcquired,
100+
bool hasCurrentUserAccessOnly,
101+
bool setStickyFlag,
99102
bool createIfNotExist,
100103
bool isSystemDirectory)
101104
{
102105
_ASSERTE(path != nullptr);
103106
_ASSERTE(!(isSystemDirectory && createIfNotExist)); // should not create or change permissions on system directories
104107
_ASSERTE(SharedMemoryManager::IsCreationDeletionProcessLockAcquired());
105108
_ASSERTE(!isGlobalLockAcquired || SharedMemoryManager::IsCreationDeletionFileLockAcquired());
109+
_ASSERTE(!(setStickyFlag && hasCurrentUserAccessOnly)); // Sticky bit doesn't make sense with current user access only
110+
111+
mode_t mode = hasCurrentUserAccessOnly ? PermissionsMask_CurrentUser_ReadWriteExecute : PermissionsMask_AllUsers_ReadWriteExecute;
112+
if (setStickyFlag)
113+
{
114+
mode |= S_ISVTX;
115+
}
106116

107117
// Check if the path already exists
108118
struct stat statInfo;
@@ -123,11 +133,11 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
123133

124134
if (isGlobalLockAcquired)
125135
{
126-
if (mkdir(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
136+
if (mkdir(path, mode) != 0)
127137
{
128138
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
129139
}
130-
if (chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
140+
if (chmod(path, mode) != 0)
131141
{
132142
rmdir(path);
133143
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
@@ -142,7 +152,7 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
142152
{
143153
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
144154
}
145-
if (chmod(tempPath, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
155+
if (chmod(tempPath, mode) != 0)
146156
{
147157
rmdir(tempPath);
148158
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
@@ -182,13 +192,18 @@ bool SharedMemoryHelpers::EnsureDirectoryExists(
182192
// For non-system directories (such as gSharedFilesPath/SHARED_MEMORY_RUNTIME_TEMP_DIRECTORY_NAME),
183193
// require sufficient permissions for all users and try to update them if requested to create the directory, so that
184194
// shared memory files may be shared by all processes on the system.
185-
if ((statInfo.st_mode & PermissionsMask_AllUsers_ReadWriteExecute) == PermissionsMask_AllUsers_ReadWriteExecute)
195+
if ((statInfo.st_mode & mode) == mode)
186196
{
187197
return true;
188198
}
189-
if (!createIfNotExist || chmod(path, PermissionsMask_AllUsers_ReadWriteExecute) != 0)
199+
if (!createIfNotExist || chmod(path, mode) != 0)
190200
{
191-
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
201+
// We were not asked to create the path or we weren't able to set the new permissions.
202+
// As a last resort, check that at least the current user has full access.
203+
if ((statInfo.st_mode & PermissionsMask_CurrentUser_ReadWriteExecute) != PermissionsMask_CurrentUser_ReadWriteExecute)
204+
{
205+
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
206+
}
192207
}
193208
return true;
194209
}
@@ -238,7 +253,7 @@ int SharedMemoryHelpers::OpenDirectory(LPCSTR path)
238253
return fileDescriptor;
239254
}
240255

241-
int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool *createdRef)
256+
int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bool isSessionScope, bool *createdRef)
242257
{
243258
_ASSERTE(path != nullptr);
244259
_ASSERTE(path[0] != '\0');
@@ -268,12 +283,13 @@ int SharedMemoryHelpers::CreateOrOpenFile(LPCSTR path, bool createIfNotExist, bo
268283

269284
// File does not exist, create the file
270285
openFlags |= O_CREAT | O_EXCL;
271-
fileDescriptor = Open(path, openFlags, PermissionsMask_AllUsers_ReadWrite);
286+
mode_t mode = isSessionScope ? PermissionsMask_CurrentUser_ReadWrite : PermissionsMask_AllUsers_ReadWrite;
287+
fileDescriptor = Open(path, openFlags, mode);
272288
_ASSERTE(fileDescriptor != -1);
273289

274290
// The permissions mask passed to open() is filtered by the process' permissions umask, so open() may not set all of
275291
// the requested permissions. Use chmod() to set the proper permissions.
276-
if (chmod(path, PermissionsMask_AllUsers_ReadWrite) != 0)
292+
if (chmod(path, mode) != 0)
277293
{
278294
CloseFile(fileDescriptor);
279295
unlink(path);
@@ -659,7 +675,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
659675
SharedMemoryHelpers::VerifyStringOperation(SharedMemoryManager::CopySharedMemoryBasePath(filePath));
660676
SharedMemoryHelpers::VerifyStringOperation(filePath.Append('/'));
661677
SharedMemoryHelpers::VerifyStringOperation(id.AppendSessionDirectoryName(filePath));
662-
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, createIfNotExist))
678+
if (!SharedMemoryHelpers::EnsureDirectoryExists(filePath, true /* isGlobalLockAcquired */, id.IsSessionScope(), false /* setStickyFlag */, createIfNotExist))
663679
{
664680
_ASSERTE(!createIfNotExist);
665681
return nullptr;
@@ -672,7 +688,7 @@ SharedMemoryProcessDataHeader *SharedMemoryProcessDataHeader::CreateOrOpen(
672688
SharedMemoryHelpers::VerifyStringOperation(filePath.Append(id.GetName(), id.GetNameCharCount()));
673689

674690
bool createdFile;
675-
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, &createdFile);
691+
int fileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(filePath, createIfNotExist, id.IsSessionScope(), &createdFile);
676692
if (fileDescriptor == -1)
677693
{
678694
_ASSERTE(!createIfNotExist);
@@ -1147,17 +1163,23 @@ void SharedMemoryManager::AcquireCreationDeletionFileLock()
11471163
if (!SharedMemoryHelpers::EnsureDirectoryExists(
11481164
*gSharedFilesPath,
11491165
false /* isGlobalLockAcquired */,
1166+
false /* hasCurrentUserAccessOnly */,
1167+
true /* setStickyFlag */,
11501168
false /* createIfNotExist */,
11511169
true /* isSystemDirectory */))
11521170
{
11531171
throw SharedMemoryException(static_cast<DWORD>(SharedMemoryError::IO));
11541172
}
11551173
SharedMemoryHelpers::EnsureDirectoryExists(
11561174
*s_runtimeTempDirectoryPath,
1157-
false /* isGlobalLockAcquired */);
1175+
false /* isGlobalLockAcquired */,
1176+
false /* hasCurrentUserAccessOnly */,
1177+
false /* setStickyFlag */);
11581178
SharedMemoryHelpers::EnsureDirectoryExists(
11591179
*s_sharedMemoryDirectoryPath,
1160-
false /* isGlobalLockAcquired */);
1180+
false /* isGlobalLockAcquired */,
1181+
false /* hasCurrentUserAccessOnly */,
1182+
true /* setStickyFlag */);
11611183
s_creationDeletionLockFileDescriptor = SharedMemoryHelpers::OpenDirectory(*s_sharedMemoryDirectoryPath);
11621184
if (s_creationDeletionLockFileDescriptor == -1)
11631185
{

‎src/coreclr/pal/src/synchobj/mutex.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,7 +1115,7 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
11151115
SharedMemoryHelpers::BuildSharedFilesPath(lockFilePath, SHARED_MEMORY_LOCK_FILES_DIRECTORY_NAME);
11161116
if (created)
11171117
{
1118-
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
1118+
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, false /* hasCurrentUserAccessOnly */, true /* setStickyFlag */);
11191119
}
11201120

11211121
// Create the session directory
@@ -1124,15 +1124,15 @@ SharedMemoryProcessDataHeader *NamedMutexProcessData::CreateOrOpen(
11241124
SharedMemoryHelpers::VerifyStringOperation(id->AppendSessionDirectoryName(lockFilePath));
11251125
if (created)
11261126
{
1127-
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */);
1127+
SharedMemoryHelpers::EnsureDirectoryExists(lockFilePath, true /* isGlobalLockAcquired */, id->IsSessionScope(), false /* setStickyFlag */);
11281128
autoCleanup.m_lockFilePath = &lockFilePath;
11291129
autoCleanup.m_sessionDirectoryPathCharCount = lockFilePath.GetCount();
11301130
}
11311131

11321132
// Create or open the lock file
11331133
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append('/'));
11341134
SharedMemoryHelpers::VerifyStringOperation(lockFilePath.Append(id->GetName(), id->GetNameCharCount()));
1135-
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created);
1135+
int lockFileDescriptor = SharedMemoryHelpers::CreateOrOpenFile(lockFilePath, created, id->IsSessionScope());
11361136
if (lockFileDescriptor == -1)
11371137
{
11381138
_ASSERTE(!created);

0 commit comments

Comments
 (0)