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

this is a collection of the trivial nits while I wait on a reply to the hard questions

Browse source
This commit is contained in:
Mike Griese 2021-11-04 12:48:45 -05:00
parent 9b4ae9ec55
commit 4976a091a0
3 changed files with 32 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

33
src/cascadia/TerminalApp/AppLogic.cpp
View file

@ -133,18 +133,6 @@ static Documents::Run _BuildErrorRun(const winrt::hstring& text, const ResourceD
return textRun;
}
// Method Description:
// - Returns whether the user is either a member of the Administrators group or
// is currently elevated.
// - This will return **FALSE** if the user has UAC disabled entirely, because
// there's no separation of power between the user and an admin in that case.
// Return Value:
// - true if the user is an administrator
static bool _isUserAdmin() noexcept
{
return Microsoft::Console::Utils::IsElevated();
}
namespace winrt::TerminalApp::implementation
{
// Function Description:
@ -196,7 +184,7 @@ namespace winrt::TerminalApp::implementation
// The TerminalPage has to be constructed during our construction, to
// make sure that there's a terminal page for callers of
// SetTitleBarContent
_isElevated = _isUserAdmin();
_isElevated = Microsoft::Console::Utils::IsElevated();
_root = winrt::make_self<TerminalPage>();
_reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {
@ -898,11 +886,20 @@ namespace winrt::TerminalApp::implementation
// actual file you wrote. So listen for that too.
wil::FolderChangeEvents::FileName | wil::FolderChangeEvents::LastWriteTime,
[this, settingsBasename = settingsPath.filename()](wil::FolderChangeEvent, PCWSTR fileModified) {
// DO NOT create a static reference to
// ApplicationState::SharedInstance here. See
// https://github.com/microsoft/terminal/pull/11222/files/9ff2775122a496fb8b1bcc7a0b83a64ce5b26c5f#r719627541
// for why. ApplicationState::SharedInstance already caches it's
// own static ref.
// DO NOT create a static reference to ApplicationState::SharedInstance here.
//
// ApplicationState::SharedInstance already caches its own
// static ref. If _we_ keep a static ref to the member in
// AppState, then our reference will keep ApplicationState alive
// after the `ActionToStringMap` gets cleaned up. Then, when we
// try to persist the actions in the window state, we won't be
// able to. We'll try to look up the action and the map just
// won't exist. We'll explode, even though the Terminal is
// tearing down anyways. So we'll just die, but still inboke
// WinDBG's post-mortem debugger, who won't be able to attach to
// the process that's already exiting.
//
// So DON'T ~give a mouse a cookie~ take a static ref here.
const winrt::hstring modifiedBasename{ std::filesystem::path{ fileModified }.filename().c_str() };

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

@ -41,7 +41,17 @@ namespace winrt::Microsoft::Terminal::Settings::Model
return baseSettingsPath;
}
static bool _hasExpectedPermissions(const std::filesystem::path& path)
// Function Description:
// - Checks the permissions on this file, to make sure it can only be opened
// for writing by admins. We want to make sure that:
// * Everyone has permission to read
// * Admins can do anything
// * No one else can do anything.
// Arguments:
// - path: the path to the file to check
// Return Value:
// - true if it had the expected permissions. False otherwise.
static bool _hasElevatedOnlyPermissions(const std::filesystem::path& path)
{
// If we want to only open the file if it's elevated, check the
// permissions on this file. We want to make sure that:
@ -74,17 +84,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model
// Now, get the Everyone and Admins SIDS so we can make sure they're
// the ones in this file.
wil::unique_sid everyoneSid;
wil::unique_sid adminGroupSid;
SID_IDENTIFIER_AUTHORITY SIDAuthNT = SECURITY_NT_AUTHORITY;
SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_WORLD_SID_AUTHORITY;
// 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, &adminGroupSid));
const auto adminGroupSid = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
// 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, &everyoneSid));
const auto everyoneSid = wil::make_static_sid(SECURITY_WORLD_SID_AUTHORITY, SECURITY_WORLD_RID);
bool hadExpectedPermissions = true;
@ -114,7 +118,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model
{
if (elevatedOnly)
{
const bool hadExpectedPermissions{ _hasExpectedPermissions(path) };
const bool hadExpectedPermissions{ _hasElevatedOnlyPermissions(path) };
if (!hadExpectedPermissions)
{
// delete the file. It's been compromised.
@ -206,16 +210,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model
// SYSTEM, but if I did that, then even we can't write the file
// while elevated, which isn't what we want.
wil::unique_sid everyoneSid;
wil::unique_sid adminGroupSid;
SID_IDENTIFIER_AUTHORITY SIDAuthNT = SECURITY_NT_AUTHORITY;
SID_IDENTIFIER_AUTHORITY SIDAuthWorld = SECURITY_WORLD_SID_AUTHORITY;
// 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, &adminGroupSid));
const auto adminGroupSid = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
// 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, &everyoneSid));
const auto everyoneSid = wil::make_static_sid(SECURITY_WORLD_SID_AUTHORITY, SECURITY_WORLD_RID);
EXPLICIT_ACCESS ea[2]{};

2
src/types/utils.cpp
View file

@ -5,7 +5,7 @@
#include "inc/utils.hpp"
#include "inc/colorTable.hpp"
#include <wil/token_helpers.h >
#include <wil/token_helpers.h>
using namespace Microsoft::Console;