commit 8abc3252b4cf9109e370eff2bd9528e12b4e03ed
parent 83cba4835230e168dc9e9c5537b4bbb5934e2ddb
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Thu, 14 Jul 2022 10:49:16 -0700
Explicit ACLs (#2324)
Explicitly apply ACLs to some locations. All are consistent with the default owners of these locations (on the current versions of Windows supported).
The locations being affected are:
- Temp :: Giving the current user full control (this has affected users in the past if their temp location did not allow Execute)
- SecureSettings :: Admins own and user can read. This is how it is now, but being explicit.
- LocalState/StandardSettings/UserFileSettings :: Current user owns. Again, not changed but being explicit.
Diffstat:
12 files changed, 521 insertions(+), 217 deletions(-)
diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt
@@ -1,7 +1,8 @@
abcd
accepteula
-adjacents
+acl
activatable
+adjacents
adml
admx
affle
@@ -93,6 +94,7 @@ ctc
Ctx
curated
CYRL
+DACL
Dbg
debian
deigh
@@ -227,6 +229,7 @@ localizationpriority
LPBYTE
LPDWORD
LPITEMIDLIST
+LPWCH
LPWSTR
LSTATUS
LTDA
@@ -287,6 +290,7 @@ ofile
Outptr
OSVERSION
Packagedx
+PACL
packageinuse
parametermap
pathparts
@@ -315,6 +319,7 @@ pscustomobject
pseudocode
psm
psobject
+ptstr
pvk
pvm
pwabuilder
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
@@ -216,6 +216,7 @@
<ClCompile Include="RestHelper.cpp" />
<ClCompile Include="RestInterface_1_0.cpp" />
<ClCompile Include="RestInterface_1_1.cpp" />
+ <ClCompile Include="Runtime.cpp" />
<ClCompile Include="SearchRequestSerializer.cpp" />
<ClCompile Include="SQLiteIndexSource.cpp" />
<ClCompile Include="Strings.cpp" />
diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters
@@ -36,7 +36,7 @@
</Filter>
<Filter Include="Source Files\Repository">
<UniqueIdentifier>{13d4d227-0f04-4e57-a663-c3c535438ab3}</UniqueIdentifier>
- </Filter>
+ </Filter>
</ItemGroup>
<ItemGroup>
<ClInclude Include="pch.h">
@@ -209,8 +209,11 @@
<ClCompile Include="YamlManifest.cpp">
<Filter>Source Files\Common</Filter>
</ClCompile>
+ <ClCompile Include="Runtime.cpp">
+ <Filter>Source Files\Common</Filter>
+ </ClCompile>
<ClCompile Include="Archive.cpp">
- <Filter>Source Files</Filter>
+ <Filter>Source Files\Common</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
@@ -602,7 +605,7 @@
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\ManifestV1_3-Singleton.yaml">
<Filter>TestData</Filter>
- </CopyFileToFolders>
+ </CopyFileToFolders>
<CopyFileToFolders Include="TestData\MultiFileManifestV1\ManifestV1-MultiFile-DefaultLocale.yaml">
<Filter>TestData\MultiFileManifestV1</Filter>
</CopyFileToFolders>
@@ -650,7 +653,7 @@
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\MultiFileManifestV1_3\ManifestV1_3-MultiFile-Version.yaml">
<Filter>TestData\MultiFileManifestV1_3</Filter>
- </CopyFileToFolders>
+ </CopyFileToFolders>
<CopyFileToFolders Include="TestData\Installer_Exe_Dependencies.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
diff --git a/src/AppInstallerCLITests/Runtime.cpp b/src/AppInstallerCLITests/Runtime.cpp
@@ -0,0 +1,83 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+#include "pch.h"
+#include "TestCommon.h"
+#include <AppInstallerRuntime.h>
+
+using namespace AppInstaller;
+using namespace AppInstaller::Runtime;
+using namespace TestCommon;
+
+
+bool CanWriteToPath(const std::filesystem::path& directory, const std::filesystem::path& file = "test.txt")
+{
+ std::ofstream out{ directory / file };
+ out << "Test";
+ return out.good();
+}
+
+void RequireAdminOwner(const std::filesystem::path& directory)
+{
+ wil::unique_hlocal_security_descriptor securityDescriptor;
+ PSID ownerSID = nullptr;
+ THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW(directory.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSID, nullptr, nullptr, nullptr, &securityDescriptor));
+
+ auto adminSID = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
+ REQUIRE(EqualSid(adminSID.get(), ownerSID));
+}
+
+TEST_CASE("ApplyACL_CurrentUserOwner", "[runtime]")
+{
+ TempDirectory directory("CurrentUserOwner");
+ PathDetails details;
+ details.Path = directory;
+ details.CurrentUser = ACEPermissions::Owner;
+
+ details.ApplyACL();
+
+ REQUIRE(CanWriteToPath(directory));
+}
+
+TEST_CASE("ApplyACL_RemoveWriteForUser", "[runtime]")
+{
+ TempDirectory directory("CurrentUserCantWrite");
+ PathDetails details;
+ details.Path = directory;
+ details.CurrentUser = ACEPermissions::ReadExecute;
+
+ details.ApplyACL();
+
+ REQUIRE(!CanWriteToPath(directory));
+}
+
+TEST_CASE("ApplyACL_AdminOwner", "[runtime]")
+{
+ TempDirectory directory("AdminOwner");
+ PathDetails details;
+ details.Path = directory;
+ details.Admins = ACEPermissions::Owner;
+
+ if (IsRunningAsAdmin())
+ {
+ details.ApplyACL();
+ RequireAdminOwner(directory);
+ REQUIRE(CanWriteToPath(directory));
+ }
+ else
+ {
+ // A non-admin token cannot set the owner to be the Admins group
+ REQUIRE_THROWS_HR(details.ApplyACL(), HRESULT_FROM_WIN32(ERROR_INVALID_OWNER));
+ }
+}
+
+TEST_CASE("ApplyACL_BothOwners", "[runtime]")
+{
+ TempDirectory directory("AdminOwner");
+ PathDetails details;
+ details.Path = directory;
+ details.CurrentUser = ACEPermissions::Owner;
+ details.Admins = ACEPermissions::Owner;
+
+ // Both cannot be owners
+ REQUIRE_THROWS_HR(details.ApplyACL(), HRESULT_FROM_WIN32(ERROR_INVALID_STATE));
+}
diff --git a/src/AppInstallerCLITests/Settings.cpp b/src/AppInstallerCLITests/Settings.cpp
@@ -160,7 +160,7 @@ TEST_CASE("SetAndReadSecureSetting_SecureDataRemoved", "[settings]")
std::string settingValue = ReadEntireStream(*result);
REQUIRE(value == settingValue);
- std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Name);
+ std::filesystem::remove(GetPathTo(PathName::SecureSettingsForRead) / name.Name);
REQUIRE_THROWS_HR(stream.Get(), SPAPI_E_FILE_HASH_NOT_IN_CATALOG);
}
diff --git a/src/AppInstallerCLITests/TestHooks.h b/src/AppInstallerCLITests/TestHooks.h
@@ -20,6 +20,7 @@ namespace AppInstaller
namespace Runtime
{
void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path);
+ void TestHook_SetPathOverride(PathName target, const PathDetails& details);
void TestHook_ClearPathOverrides();
}
diff --git a/src/AppInstallerCLITests/main.cpp b/src/AppInstallerCLITests/main.cpp
@@ -154,7 +154,8 @@ int main(int argc, char** argv)
Runtime::TestHook_SetPathOverride(Runtime::PathName::LocalState, Runtime::GetPathTo(Runtime::PathName::LocalState) / "Tests");
Runtime::TestHook_SetPathOverride(Runtime::PathName::UserFileSettings, Runtime::GetPathTo(Runtime::PathName::UserFileSettings) / "Tests");
Runtime::TestHook_SetPathOverride(Runtime::PathName::StandardSettings, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "Tests");
- Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettings, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "WinGet_SecureSettings_Tests");
+ Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForRead, Runtime::GetPathTo(Runtime::PathName::StandardSettings) / "WinGet_SecureSettings_Tests");
+ Runtime::TestHook_SetPathOverride(Runtime::PathName::SecureSettingsForWrite, Runtime::GetPathDetailsFor(Runtime::PathName::SecureSettingsForRead));
int result = Catch::Session().run(static_cast<int>(args.size()), args.data());
diff --git a/src/AppInstallerCLITests/pch.h b/src/AppInstallerCLITests/pch.h
@@ -3,6 +3,7 @@
#pragma once
#define NOMINMAX
#include <Windows.h>
+#include <AclAPI.h>
#include <WinInet.h>
#include <shellapi.h>
#include <objbase.h>
@@ -18,6 +19,7 @@
#include <wil/resource.h>
#include <wil/result_macros.h>
+#include <wil/token_helpers.h>
#include <atomic>
#include <filesystem>
diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h
@@ -27,6 +27,9 @@ namespace AppInstaller::Runtime
// This can be used as the current market.
std::string GetOSRegion();
+ // Sets the runtime path state name globally.
+ void SetRuntimePathStateName(std::string name);
+
// A path to be retrieved based on the runtime.
enum class PathName
{
@@ -43,8 +46,10 @@ namespace AppInstaller::Runtime
StandardSettings,
// The location that user file type settings are stored.
UserFileSettings,
- // The location where secure settings data is stored.
- SecureSettings,
+ // The location where secure settings data is stored (for reading).
+ SecureSettingsForRead,
+ // The location where secure settings data is stored (for writing).
+ SecureSettingsForWrite,
// The value of %USERPROFILE%.
UserProfile,
// The location where portable packages are installed to with user scope.
@@ -59,7 +64,42 @@ namespace AppInstaller::Runtime
PortableLinksMachineLocation,
};
- void SetRuntimePathStateName(std::string name);
+ // The permissions granted to a specific ACE.
+ enum class ACEPermissions : uint32_t
+ {
+ // This is not "Deny All", but rather, "Not mentioned"
+ None = 0x0,
+ Read = 0x1,
+ Write = 0x2,
+ Execute = 0x4,
+ ReadWrite = Read | Write,
+ ReadExecute = Read | Execute,
+ ReadWriteExecute = Read | Write | Execute,
+ // Owner means that full control will be granted
+ Owner = 0xFFFFFFFF
+ };
+
+ DEFINE_ENUM_FLAG_OPERATORS(ACEPermissions);
+
+ // Information about a path that we use and how to set it up.
+ struct PathDetails
+ {
+ std::filesystem::path Path;
+ // Default to creating the directory with inherited permissions
+ bool Create = true;
+ ACEPermissions CurrentUser = ACEPermissions::None;
+ ACEPermissions Admins = ACEPermissions::None;
+
+ // Determines if the ACL should be applied.
+ bool ShouldApplyACL() const;
+
+ // Applies the ACL unconditionally.
+ void ApplyACL() const;
+ };
+
+ // Gets the PathDetails used for the given path.
+ // This is exposed primarily to allow for testing, GetPathTo should be preferred.
+ PathDetails GetPathDetailsFor(PathName path);
// Gets the path to the requested location.
std::filesystem::path GetPathTo(PathName path);
diff --git a/src/AppInstallerCommonCore/Runtime.cpp b/src/AppInstallerCommonCore/Runtime.cpp
@@ -2,11 +2,12 @@
// Licensed under the MIT License.
#include "pch.h"
#include <binver/version.h>
+#include "Public/AppInstallerLogging.h"
#include "Public/AppInstallerRuntime.h"
#include "Public/AppInstallerStrings.h"
+#include "Public/winget/Filesystem.h"
#include "Public/winget/UserSettings.h"
-#include <optional>
#define WINGET_DEFAULT_LOG_DIRECTORY "DiagOutputDir"
@@ -89,7 +90,7 @@ namespace AppInstaller::Runtime
}
#ifndef AICLI_DISABLE_TEST_HOOKS
- static std::map<PathName, std::filesystem::path> s_Path_TestHook_Overrides;
+ static std::map<PathName, PathDetails> s_Path_TestHook_Overrides;
#endif
std::filesystem::path GetKnownFolderPath(const KNOWNFOLDERID& id)
@@ -122,7 +123,6 @@ namespace AppInstaller::Runtime
}
// Gets the path to the app data relative directory.
- // Creates the directory if it does not already exist.
std::filesystem::path GetPathToAppDataDir(const std::filesystem::path& relative)
{
THROW_HR_IF(E_INVALIDARG, !relative.has_relative_path());
@@ -162,6 +162,66 @@ namespace AppInstaller::Runtime
return result;
}
+
+ // If `source` begins with all of `prefix`, replace that with `replacement`.
+ void ReplaceCommonPathPrefix(std::filesystem::path& source, const std::filesystem::path& prefix, std::string_view replacement)
+ {
+ auto prefixItr = prefix.begin();
+ auto sourceItr = source.begin();
+
+ while (prefixItr != prefix.end() && sourceItr != source.end())
+ {
+ if (*prefixItr != *sourceItr)
+ {
+ break;
+ }
+
+ ++prefixItr;
+ ++sourceItr;
+ }
+
+ // Only replace source if we found all of prefix
+ if (prefixItr == prefix.end())
+ {
+ std::filesystem::path temp{ replacement };
+
+ for (; sourceItr != source.end(); ++sourceItr)
+ {
+ temp /= *sourceItr;
+ }
+
+ source = std::move(temp);
+ }
+ }
+
+ DWORD AccessPermissionsFrom(ACEPermissions permissions)
+ {
+ DWORD result = 0;
+
+ if (permissions == ACEPermissions::Owner)
+ {
+ result |= GENERIC_ALL;
+ }
+ else
+ {
+ if (WI_IsFlagSet(permissions, ACEPermissions::Read))
+ {
+ result |= GENERIC_READ;
+ }
+
+ if (WI_IsFlagSet(permissions, ACEPermissions::Write))
+ {
+ result |= GENERIC_WRITE | FILE_DELETE_CHILD;
+ }
+
+ if (WI_IsFlagSet(permissions, ACEPermissions::Execute))
+ {
+ result |= GENERIC_EXECUTE;
+ }
+ }
+
+ return result;
+ }
}
bool IsRunningInPackagedContext()
@@ -269,217 +329,284 @@ namespace AppInstaller::Runtime
s_runtimePathStateName.emplace(std::move(suitablePathPart));
}
- std::filesystem::path GetPathTo(PathName path)
+ bool PathDetails::ShouldApplyACL() const
{
- std::filesystem::path result;
- bool create = true;
+ // Could be expanded to actually check the current owner/ACL on the path, but isn't worth it currently
+ return (CurrentUser != ACEPermissions::None || Admins != ACEPermissions::None);
+ }
-#ifndef WINGET_DISABLE_FOR_FUZZING
- if (IsRunningInPackagedContext())
+ void PathDetails::ApplyACL() const
+ {
+ THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_INVALID_STATE), CurrentUser == ACEPermissions::Owner && Admins == ACEPermissions::Owner);
+
+ ULONG entriesCount = 0;
+ EXPLICIT_ACCESS_W explicitAccess[2];
+
+ decltype(wil::get_token_information<TOKEN_USER>()) userToken;
+ auto adminSID = wil::make_static_sid(SECURITY_NT_AUTHORITY, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS);
+ PSID ownerSID = nullptr;
+
+ if (CurrentUser != ACEPermissions::None)
{
- auto appStorage = winrt::Windows::Storage::ApplicationData::Current();
+ userToken = wil::get_token_information<TOKEN_USER>();
- switch (path)
+ if (CurrentUser == ACEPermissions::Owner)
{
- case PathName::Temp:
+ ownerSID = userToken->User.Sid;
+ }
+
+ EXPLICIT_ACCESS_W& entry = explicitAccess[entriesCount++];
+ entry = {};
+
+ entry.grfAccessPermissions = AccessPermissionsFrom(CurrentUser);
+ entry.grfAccessMode = SET_ACCESS;
+ entry.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;
+
+ entry.Trustee.pMultipleTrustee = nullptr;
+ entry.Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
+ entry.Trustee.TrusteeForm = TRUSTEE_IS_SID;
+ entry.Trustee.TrusteeType = TRUSTEE_IS_USER;
+ entry.Trustee.ptstrName = reinterpret_cast<LPWCH>(userToken->User.Sid);
+ }
+
+ if (Admins != ACEPermissions::None)
+ {
+ if (Admins == ACEPermissions::Owner)
{
- result = GetPathToUserTemp();
- result /= s_DefaultTempDirectory;
+ ownerSID = adminSID.get();
}
- break;
- case PathName::LocalState:
- case PathName::UserFileSettings:
- result.assign(appStorage.LocalFolder().Path().c_str());
- break;
- case PathName::DefaultLogLocation:
- case PathName::DefaultLogLocationForDisplay:
- // To enable UIF collection through Feedback hub, we must put our logs here.
- result.assign(appStorage.LocalFolder().Path().c_str());
- result /= WINGET_DEFAULT_LOG_DIRECTORY;
-
- if (path == PathName::DefaultLogLocationForDisplay)
- {
- std::filesystem::path localAppData = GetKnownFolderPath(FOLDERID_LocalAppData);
-
- auto ladItr = localAppData.begin();
- auto resultItr = result.begin();
-
- while (ladItr != localAppData.end() && resultItr != result.end())
- {
- if (*ladItr != *resultItr)
- {
- break;
- }
-
- ++ladItr;
- ++resultItr;
- }
-
- if (ladItr == localAppData.end())
- {
- localAppData.assign("%LOCALAPPDATA%");
-
- for (;resultItr != result.end(); ++resultItr)
- {
- localAppData /= *resultItr;
- }
-
- result = std::move(localAppData);
- }
- }
- break;
- case PathName::StandardSettings:
- create = false;
- break;
- case PathName::SecureSettings:
- result = GetKnownFolderPath(FOLDERID_ProgramData);
- result /= s_SecureSettings_Base;
- result /= GetUserSID();
- result /= s_SecureSettings_UserRelative;
- result /= s_SecureSettings_Relative_Packaged;
- result /= GetPackageName();
- create = false;
- break;
- case PathName::UserProfile:
- result = GetKnownFolderPath(FOLDERID_Profile);
- create = false;
- break;
- case PathName::PortablePackageUserRoot:
- result = Settings::User().Get<Setting::PortableAppUserRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_LocalAppData);
- result /= s_PortablePackageUserRoot_Base;
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortablePackageMachineRootX64:
- result = Settings::User().Get<Setting::PortableAppMachineRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortablePackageMachineRootX86:
- result = Settings::User().Get<Setting::PortableAppMachineRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX86);
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortableLinksUserLocation:
- result = GetKnownFolderPath(FOLDERID_LocalAppData);
- result /= s_PortablePackageUserRoot_Base;
- result /= s_PortablePackageRoot;
- result /= s_LinksDirectory;
- create = true;
- break;
- case PathName::PortableLinksMachineLocation:
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
- result /= s_PortablePackageRoot;
- result /= s_LinksDirectory;
- create = true;
- break;
- default:
- THROW_HR(E_UNEXPECTED);
+
+ EXPLICIT_ACCESS_W& entry = explicitAccess[entriesCount++];
+ entry = {};
+
+ entry.grfAccessPermissions = AccessPermissionsFrom(Admins);
+ entry.grfAccessMode = SET_ACCESS;
+ entry.grfInheritance = CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE;
+
+ entry.Trustee.pMultipleTrustee = nullptr;
+ entry.Trustee.MultipleTrusteeOperation = NO_MULTIPLE_TRUSTEE;
+ entry.Trustee.TrusteeForm = TRUSTEE_IS_SID;
+ entry.Trustee.TrusteeType = TRUSTEE_IS_WELL_KNOWN_GROUP;
+ entry.Trustee.ptstrName = reinterpret_cast<LPWCH>(adminSID.get());
+ }
+
+ wil::unique_any<PACL, decltype(&::LocalFree), ::LocalFree> acl;
+ THROW_IF_WIN32_ERROR(SetEntriesInAclW(entriesCount, explicitAccess, nullptr, &acl));
+
+ std::wstring path = Path.wstring();
+ SECURITY_INFORMATION securityInformation = DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION;
+
+ if (ownerSID)
+ {
+ securityInformation |= OWNER_SECURITY_INFORMATION;
+ }
+
+ THROW_IF_WIN32_ERROR(SetNamedSecurityInfoW(&path[0], SE_FILE_OBJECT, securityInformation, ownerSID, nullptr, acl.get(), nullptr));
+ }
+
+ // Contains all of the paths that are common between the runtime contexts.
+ PathDetails GetPathDetailsCommon(PathName path)
+ {
+ PathDetails result;
+
+ switch (path)
+ {
+ case PathName::UserProfile:
+ result.Path = GetKnownFolderPath(FOLDERID_Profile);
+ result.Create = false;
+ break;
+ case PathName::PortablePackageUserRoot:
+ result.Path = Settings::User().Get<Setting::PortableAppUserRoot>();
+ if (result.Path.empty())
+ {
+ result.Path = GetKnownFolderPath(FOLDERID_LocalAppData);
+ result.Path /= s_PortablePackageUserRoot_Base;
+ result.Path /= s_PortablePackageRoot;
+ result.Path /= s_PortablePackagesDirectory;
+ }
+ break;
+ case PathName::PortablePackageMachineRootX64:
+ result.Path = Settings::User().Get<Setting::PortableAppMachineRoot>();
+ if (result.Path.empty())
+ {
+ result.Path = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
+ result.Path /= s_PortablePackageRoot;
+ result.Path /= s_PortablePackagesDirectory;
+ }
+ break;
+ case PathName::PortablePackageMachineRootX86:
+ result.Path = Settings::User().Get<Setting::PortableAppMachineRoot>();
+ if (result.Path.empty())
+ {
+ result.Path = GetKnownFolderPath(FOLDERID_ProgramFilesX86);
+ result.Path /= s_PortablePackageRoot;
+ result.Path /= s_PortablePackagesDirectory;
}
+ break;
+ case PathName::PortableLinksUserLocation:
+ result.Path = GetKnownFolderPath(FOLDERID_LocalAppData);
+ result.Path /= s_PortablePackageUserRoot_Base;
+ result.Path /= s_PortablePackageRoot;
+ result.Path /= s_LinksDirectory;
+ break;
+ case PathName::PortableLinksMachineLocation:
+ result.Path = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
+ result.Path /= s_PortablePackageRoot;
+ result.Path /= s_LinksDirectory;
+ break;
+ default:
+ THROW_HR(E_UNEXPECTED);
}
- else
+
+ return result;
+ }
+
+#ifndef WINGET_DISABLE_FOR_FUZZING
+ PathDetails GetPathDetailsForPackagedContext(PathName path)
+ {
+ PathDetails result;
+
+ auto appStorage = winrt::Windows::Storage::ApplicationData::Current();
+
+ switch (path)
+ {
+ case PathName::Temp:
+ result.Path = GetPathToUserTemp() / s_DefaultTempDirectory;
+ result.CurrentUser = ACEPermissions::Owner;
+ break;
+ case PathName::LocalState:
+ case PathName::UserFileSettings:
+ result.Path.assign(appStorage.LocalFolder().Path().c_str());
+ break;
+ case PathName::DefaultLogLocation:
+ case PathName::DefaultLogLocationForDisplay:
+ // To enable UIF collection through Feedback hub, we must put our logs here.
+ result.Path.assign(appStorage.LocalFolder().Path().c_str());
+ result.Path /= WINGET_DEFAULT_LOG_DIRECTORY;
+
+ if (path == PathName::DefaultLogLocationForDisplay)
+ {
+ ReplaceCommonPathPrefix(result.Path, GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%");
+ }
+ break;
+ case PathName::StandardSettings:
+ result.Create = false;
+ break;
+ case PathName::SecureSettingsForRead:
+ case PathName::SecureSettingsForWrite:
+ result.Path = GetKnownFolderPath(FOLDERID_ProgramData);
+ result.Path /= s_SecureSettings_Base;
+ result.Path /= GetUserSID();
+ result.Path /= s_SecureSettings_UserRelative;
+ result.Path /= s_SecureSettings_Relative_Packaged;
+ result.Path /= GetPackageName();
+ if (path == PathName::SecureSettingsForWrite)
+ {
+ result.Admins = ACEPermissions::Owner;
+ result.CurrentUser = ACEPermissions::ReadExecute;
+ }
+ else
+ {
+ result.Create = false;
+ }
+ break;
+ case PathName::UserProfile:
+ case PathName::PortablePackageUserRoot:
+ case PathName::PortablePackageMachineRootX64:
+ case PathName::PortablePackageMachineRootX86:
+ case PathName::PortableLinksUserLocation:
+ case PathName::PortableLinksMachineLocation:
+ result = GetPathDetailsCommon(path);
+ break;
+ default:
+ THROW_HR(E_UNEXPECTED);
+ }
+
+ return result;
+ }
#endif
+
+ PathDetails GetPathDetailsForUnpackagedContext(PathName path)
+ {
+ PathDetails result;
+
+ switch (path)
{
- switch (path)
+ case PathName::Temp:
+ case PathName::DefaultLogLocation:
+ {
+ result.Path = GetPathToUserTemp();
+ result.Path /= s_DefaultTempDirectory;
+ result.Path /= GetRuntimePathStateName();
+ if (path == PathName::Temp)
{
- case PathName::Temp:
- case PathName::DefaultLogLocation:
+ result.CurrentUser = ACEPermissions::Owner;
+ }
+ }
+ break;
+ case PathName::DefaultLogLocationForDisplay:
+ result.Path.assign("%TEMP%");
+ result.Path /= s_DefaultTempDirectory;
+ result.Path /= GetRuntimePathStateName();
+ result.Create = false;
+ break;
+ case PathName::LocalState:
+ result.Path = GetPathToAppDataDir(s_AppDataDir_State);
+ result.Path /= GetRuntimePathStateName();
+ result.CurrentUser = ACEPermissions::Owner;
+ break;
+ case PathName::StandardSettings:
+ case PathName::UserFileSettings:
+ result.Path = GetPathToAppDataDir(s_AppDataDir_Settings);
+ result.Path /= GetRuntimePathStateName();
+ result.CurrentUser = ACEPermissions::Owner;
+ break;
+ case PathName::SecureSettingsForRead:
+ case PathName::SecureSettingsForWrite:
+ result.Path = GetKnownFolderPath(FOLDERID_ProgramData);
+ result.Path /= s_SecureSettings_Base;
+ result.Path /= GetUserSID();
+ result.Path /= s_SecureSettings_UserRelative;
+ result.Path /= s_SecureSettings_Relative_Unpackaged;
+ result.Path /= GetRuntimePathStateName();
+ if (path == PathName::SecureSettingsForWrite)
{
- result = GetPathToUserTemp();
- result /= s_DefaultTempDirectory;
- result /= GetRuntimePathStateName();
+ result.Admins = ACEPermissions::Owner;
+ result.CurrentUser = ACEPermissions::ReadExecute;
}
- break;
- case PathName::DefaultLogLocationForDisplay:
- result.assign("%TEMP%");
- result /= s_DefaultTempDirectory;
- result /= GetRuntimePathStateName();
- create = false;
- break;
- case PathName::LocalState:
- result = GetPathToAppDataDir(s_AppDataDir_State);
- result /= GetRuntimePathStateName();
- break;
- case PathName::StandardSettings:
- case PathName::UserFileSettings:
- result = GetPathToAppDataDir(s_AppDataDir_Settings);
- result /= GetRuntimePathStateName();
- break;
- case PathName::SecureSettings:
- result = GetKnownFolderPath(FOLDERID_ProgramData);
- result /= s_SecureSettings_Base;
- result /= GetUserSID();
- result /= s_SecureSettings_UserRelative;
- result /= s_SecureSettings_Relative_Unpackaged;
- result /= GetRuntimePathStateName();
- create = false;
- break;
- case PathName::UserProfile:
- result = GetKnownFolderPath(FOLDERID_Profile);
- create = false;
- break;
- case PathName::PortablePackageUserRoot:
- result = Settings::User().Get<Setting::PortableAppUserRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_LocalAppData);
- result /= s_PortablePackageUserRoot_Base;
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortablePackageMachineRootX64:
- result = Settings::User().Get<Setting::PortableAppMachineRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortablePackageMachineRootX86:
- result = Settings::User().Get<Setting::PortableAppMachineRoot>();
- if (result.empty())
- {
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX86);
- result /= s_PortablePackageRoot;
- result /= s_PortablePackagesDirectory;
- }
- create = true;
- break;
- case PathName::PortableLinksUserLocation:
- result = GetKnownFolderPath(FOLDERID_LocalAppData);
- result /= s_PortablePackageUserRoot_Base;
- result /= s_PortablePackageRoot;
- result /= s_LinksDirectory;
- create = true;
- break;
- case PathName::PortableLinksMachineLocation:
- result = GetKnownFolderPath(FOLDERID_ProgramFilesX64);
- result /= s_PortablePackageRoot;
- result /= s_LinksDirectory;
- create = true;
- break;
- default:
- THROW_HR(E_UNEXPECTED);
+ else
+ {
+ result.Create = false;
}
+ break;
+ case PathName::UserProfile:
+ case PathName::PortablePackageUserRoot:
+ case PathName::PortablePackageMachineRootX64:
+ case PathName::PortablePackageMachineRootX86:
+ case PathName::PortableLinksUserLocation:
+ case PathName::PortableLinksMachineLocation:
+ result = GetPathDetailsCommon(path);
+ break;
+ default:
+ THROW_HR(E_UNEXPECTED);
+ }
+
+ return result;
+ }
+
+ PathDetails GetPathDetailsFor(PathName path)
+ {
+ PathDetails result;
+
+#ifndef WINGET_DISABLE_FOR_FUZZING
+ if (IsRunningInPackagedContext())
+ {
+ result = GetPathDetailsForPackagedContext(path);
+ }
+ else
+#endif
+ {
+ result = GetPathDetailsForUnpackagedContext(path);
}
#ifndef AICLI_DISABLE_TEST_HOOKS
@@ -491,17 +618,39 @@ namespace AppInstaller::Runtime
}
#endif
- if (create && result.is_absolute())
+ return result;
+ }
+
+ std::filesystem::path GetPathTo(PathName path)
+ {
+ PathDetails details = GetPathDetailsFor(path);
+
+ if (details.Create)
{
- if (std::filesystem::exists(result) && !std::filesystem::is_directory(result))
+ if (details.Path.is_absolute())
{
- std::filesystem::remove(result);
- }
+ if (std::filesystem::exists(details.Path) && !std::filesystem::is_directory(details.Path))
+ {
+ std::filesystem::remove(details.Path);
+ }
- std::filesystem::create_directories(result);
+ std::filesystem::create_directories(details.Path);
+
+ // Set the ACLs on the directory if needed. We do this after creating the directory because an attacker could
+ // have created the directory beforehand so we must be able to place the correct ACL on any directory or fail
+ // to operate.
+ if (details.ShouldApplyACL())
+ {
+ details.ApplyACL();
+ }
+ }
+ else
+ {
+ AICLI_LOG(Core, Warning, << "GetPathTo directory creation requested for [" << path << "], but path was not absolute: " << details.Path);
+ }
}
- return result;
+ return std::move(details.Path);
}
std::filesystem::path GetNewTempFilePath()
@@ -579,7 +728,21 @@ namespace AppInstaller::Runtime
#ifndef AICLI_DISABLE_TEST_HOOKS
void TestHook_SetPathOverride(PathName target, const std::filesystem::path& path)
{
- s_Path_TestHook_Overrides[target] = path;
+ if (s_Path_TestHook_Overrides.count(target))
+ {
+ s_Path_TestHook_Overrides[target].Path = path;
+ }
+ else
+ {
+ PathDetails details = GetPathDetailsFor(target);
+ details.Path = path;
+ s_Path_TestHook_Overrides[target] = std::move(details);
+ }
+ }
+
+ void TestHook_SetPathOverride(PathName target, const PathDetails& details)
+ {
+ s_Path_TestHook_Overrides[target] = details;
}
void TestHook_ClearPathOverrides()
diff --git a/src/AppInstallerCommonCore/Settings.cpp b/src/AppInstallerCommonCore/Settings.cpp
@@ -231,7 +231,7 @@ namespace AppInstaller::Settings
constexpr static std::string_view NodeName_Sha256 = "SHA256"sv;
SecureSettingsContainer(std::unique_ptr<ISettingsContainer>&& container, const std::string_view& name) :
- ExchangeSettingsContainer(std::move(container), name), m_secure(GetPathTo(PathName::SecureSettings), name) {}
+ ExchangeSettingsContainer(std::move(container), name), m_secure(GetPathTo(PathName::SecureSettingsForRead), name) {}
private:
struct VerificationData
@@ -316,6 +316,9 @@ namespace AppInstaller::Settings
bool Set(std::string_view value) override
{
+ // Force the creation of the secure settings location with appropriate ACLs
+ GetPathTo(PathName::SecureSettingsForWrite);
+
bool exchangeResult = ExchangeSettingsContainer::Set(value);
if (exchangeResult)
diff --git a/src/AppInstallerCommonCore/pch.h b/src/AppInstallerCommonCore/pch.h
@@ -4,6 +4,7 @@
#define NOMINMAX
#include <Windows.h>
+#include <AclAPI.h>
#include <appmodel.h>
#include <WinInet.h>
#include <sddl.h>
@@ -46,6 +47,7 @@
#include <limits>
#include <memory>
#include <mutex>
+#include <optional>
#include <ostream>
#include <regex>
#include <set>
