maxmustermann/terminal
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

I bet that's what I was doing wrong. The rename is just dandy, it don't care about elevation or none of that, so the unelevated terminal could atomically rewrite the elevated file, which isn't what we wanted.

Browse source
This commit is contained in:
Mike Griese 2021-09-09 12:09:06 -05:00
parent 9a1cf5ac6e
commit 7854abe0a3
2 changed files with 6 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
src/cascadia/TerminalSettingsModel/FileUtils.cpp
View file

@ -129,27 +129,15 @@ namespace Microsoft::Terminal::Settings::Model
PSID pEveryoneSid = nullptr;
PSID pAdminGroupSid = nullptr;
PSID pAdminSid = nullptr;
SID_IDENTIFIER_AUTHORITY SIDAuthNT = SECURITY_NT_AUTHORITY;
SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_WORLD_SID_AUTHORITY;
// I thought DOMAIN_USER_RID_ADMIN might be the Builtin\Admin sid, but that returned garbage
// Create a SID for the BUILTIN\Administrators group.
THROW_IF_WIN32_BOOL_FALSE(AllocateAndInitializeSid(&SIDAuthNT, 2, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS, 0, 0, 0, 0, 0, 0, &pAdminGroupSid));
// ?
THROW_IF_WIN32_BOOL_FALSE(AllocateAndInitializeSid(&SIDAuthNT, 1, DOMAIN_USER_RID_ADMIN, 0, 0, 0, 0, 0, 0, 0, &pAdminSid));
// Create a well-known SID for the Everyone group.
THROW_IF_WIN32_BOOL_FALSE(AllocateAndInitializeSid(&SIDAuthWorld, 1, SECURITY_WORLD_RID, 0, 0, 0, 0, 0, 0, 0, &pEveryoneSid));
// wil::unique_sid adminSid{};
// DWORD cbSize = sizeof(SID);
// THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinAccountAdministratorSid, nullptr, adminSid.put(), &cbSize));
// BYTE adminSid[SECURITY_MAX_SID_SIZE] = { 0 };
// DWORD adminSize = ARRAYSIZE(adminSid);
// auto success = CreateWellKnownSid(WinAccountAdministratorSid, nullptr, adminSid, &adminSize);
// THROW_IF_WIN32_BOOL_FALSE(success);
EXPLICIT_ACCESS ea[2];
ZeroMemory(&ea, 2 * sizeof(EXPLICIT_ACCESS));
// Grant Admins all permissions on this file
@ -158,7 +146,7 @@ namespace Microsoft::Terminal::Settings::Model
ea[0].grfInheritance = NO_INHERITANCE;
ea[0].Trustee.TrusteeForm = TRUSTEE_IS_SID;
ea[0].Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP;
ea[0].Trustee.ptstrName = (LPTSTR)pAdminSid;
ea[0].Trustee.ptstrName = (LPTSTR)pAdminGroupSid;
// Grant Everyone the permission or read this file
ea[1].grfAccessPermissions = GENERIC_READ;
@ -202,8 +190,7 @@ namespace Microsoft::Terminal::Settings::Model
}
void WriteUTF8FileAtomic(const std::filesystem::path& path,
const std::string_view content,
const bool elevatedOnly)
const std::string_view content)
{
// GH#10787: rename() will replace symbolic links themselves and not the path they point at.
// It's thus important that we first resolve them before generating temporary path.
@ -221,7 +208,7 @@ namespace Microsoft::Terminal::Settings::Model
// * resolve the link manually, which might be less accurate and more prone to race conditions
// * write to the file directly, which lets the system resolve the symbolic link but leaves the write non-atomic
// The latter is chosen, as this is an edge case and our 'atomic' writes are only best-effort.
WriteUTF8File(path, content, elevatedOnly);
WriteUTF8File(path, content);
return;
}
@ -229,7 +216,7 @@ namespace Microsoft::Terminal::Settings::Model
tmpPath += L".tmp";
// Writing to a file isn't atomic, but...
WriteUTF8File(tmpPath, content, elevatedOnly);
WriteUTF8File(tmpPath, content);
// renaming one is (supposed to be) atomic.
// Wait... "supposed to be"!? Well it's technically not always atomic,

2
src/cascadia/TerminalSettingsModel/FileUtils.h
View file

@ -7,5 +7,5 @@ namespace Microsoft::Terminal::Settings::Model
std::string ReadUTF8File(const std::filesystem::path& path);
std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path);
void WriteUTF8File(const std::filesystem::path& path, const std::string_view content, const bool elevatedOnly = false);
void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content, const bool elevatedOnly = false);
void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view content);
}