Skip to content

Commit 2515a35

Browse files
authored
Change symlink verification to avoid redirection guard policy (#6239)
## 📖 Description Rather than comparing the symlink and desired target through `weakly_canonical`, this change checks that the symlink is pointing to the target via string comparison. This avoids "resolving" the symlink and the redirection guard policy.
1 parent cf184b7 commit 2515a35

3 files changed

Lines changed: 27 additions & 2 deletions

File tree

src/AppInstallerCLITests/Filesystem.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ TEST_CASE("VerifySymlink", "[filesystem]")
4949
REQUIRE(VerifySymlink(symlinkPath, testFilePath));
5050
REQUIRE_FALSE(VerifySymlink(symlinkPath, "badPath"));
5151

52+
// Verify case-insensitive comparison works (Windows paths are case-insensitive).
53+
std::filesystem::path testFilePathUpperCase = basePath / "TESTFILE.TXT";
54+
REQUIRE(VerifySymlink(symlinkPath, testFilePathUpperCase));
55+
5256
std::filesystem::remove(testFilePath);
5357

5458
// Ensure that symlink existence does not check the target

src/AppInstallerCLITests/main.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ int main(int argc, char** argv)
6767
{
6868
init_apartment();
6969

70+
// Enable ProcessRedirectionTrustPolicy to match the MSIX packaged process context in which
71+
// WinGet runs in production. This policy causes Windows to block traversal of user-created
72+
// symlinks (e.g. via weakly_canonical), so enabling it here surfaces bugs that would only
73+
// manifest in the real product environment. The policy is one-way; it cannot be reversed.
74+
PROCESS_MITIGATION_REDIRECTION_TRUST_POLICY redirectionPolicy{};
75+
redirectionPolicy.EnforceRedirectionTrust = 1;
76+
SetProcessMitigationPolicy(ProcessRedirectionTrustPolicy, &redirectionPolicy, sizeof(redirectionPolicy));
77+
7078
bool hasSetTestDataBasePath = false;
7179
bool waitBeforeReturn = false;
7280
bool keepSQLLogging = false;

src/AppInstallerSharedLib/Filesystem.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,21 @@ namespace AppInstaller::Filesystem
477477

478478
bool VerifySymlink(const std::filesystem::path& symlink, const std::filesystem::path& target)
479479
{
480-
const std::filesystem::path& symlinkTargetPath = std::filesystem::weakly_canonical(symlink);
481-
return symlinkTargetPath == std::filesystem::weakly_canonical(target);
480+
// Use read_symlink to get the symlink's recorded target without traversing the filesystem
481+
// chain. weakly_canonical would follow the symlink, which is blocked by
482+
// ProcessRedirectionTrustPolicy (inherited from the MSIX packaged process context on
483+
// newer Windows builds) when the symlink was created by a non-elevated process.
484+
const std::filesystem::path symlinkTarget = std::filesystem::read_symlink(symlink);
485+
486+
// If the recorded target is relative, resolve it against the symlink's parent directory.
487+
const std::filesystem::path resolvedTarget = symlinkTarget.is_absolute() ?
488+
symlinkTarget : (symlink.parent_path() / symlinkTarget);
489+
490+
// Windows paths are case-insensitive. Use lexically_normal to resolve . and .. without
491+
// filesystem access, then compare case-insensitively.
492+
return Utility::ICUCaseInsensitiveEquals(
493+
resolvedTarget.lexically_normal().u8string(),
494+
target.lexically_normal().u8string());
482495
}
483496

484497
void AppendExtension(std::filesystem::path& target, const std::string& value)

0 commit comments

Comments
 (0)