@samitouri / QOSAMI-WSL / commits / 36ad0abb

Fix MoveDistribution E_ACCESSDENIED when setVhdOwner fails under impersonation (#40717)

* Fix MoveDistribution E_ACCESSDENIED when setVhdOwner fails under impersonation (#40716) After a cross-volume MoveFileEx, the new VHD file's owner may be set to BUILTIN\Administrators. The setVhdOwner lambda was opening the file with WRITE_OWNER under user impersonation, which fails with E_ACCESSDENIED if the user doesn't own the file. The subsequent run_as_self() came too late. Move run_as_self() and AcquirePrivilege(SE_RESTORE_NAME) before the CreateFileW call so the file is opened as SYSTEM, which always has WRITE_OWNER regardless of file ownership. Previously, this failure left the VHD at the new location with the registry BasePath still pointing at the old path, corrupting the distro. Fixes #40716 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add test for MoveDistribution with admin-owned VHD (#40716) Regression test that simulates a cross-volume move scenario: after moving a distro, the VHD owner is explicitly set to BUILTIN\Administrators (mimicking MoveFileEx cross-volume behavior), then a second move is attempted as a non-elevated user. Before the fix, setVhdOwner would fail with E_ACCESSDENIED because CreateFileW(WRITE_OWNER) ran under user impersonation. The test verifies: - The move succeeds even when the VHD is owned by Administrators - The VHD owner is restored to the user's SID after the move - The distro launches successfully after the move Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Ben Hillis committed Jun 8, 2026 at 07:16 UTC 36ad0abbe44272aedfa8db5a11b30432863ec27c
2 files changed +79 -2
src/windows/service/exe/LxssUserSession.cpp
+6 -2
@@ -952,13 +952,17 @@ HRESULT LxssUserSessionImpl::MoveDistribution(_In_ LPCGUID DistroGuid, _In_ LPCW
952 THROW_IF_WIN32_BOOL_FALSE(MoveFileEx(distro.VhdFilePath.c_str(), newVhdPath.c_str(), MOVEFILE_COPY_ALLOWED | MOVEFILE_WRITE_THROUGH));
953
954 // Restore the original VHD owner on the moved file.
955 + // Run as self (SYSTEM) for both the file open and the SetSecurityInfo call,
956 + // because after a cross-volume MoveFileEx the new file's owner may be
957 + // BUILTIN\Administrators and the impersonated user token may lack WRITE_OWNER.
958 auto setVhdOwner = [&originalOwner](const std::filesystem::path& vhdPath) {
959 + auto runAsSelf = wil::run_as_self();
960 + auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME);
961 +
962 wil::unique_hfile vhdHandle(CreateFileW(
963 vhdPath.c_str(), WRITE_OWNER, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, FILE_FLAG_OPEN_REPARSE_POINT, nullptr));
964 THROW_LAST_ERROR_IF(!vhdHandle);
965
960 - auto runAsSelf = wil::run_as_self();
961 - auto privileges = wsl::windows::common::security::AcquirePrivilege(SE_RESTORE_NAME);
966 THROW_IF_WIN32_ERROR(
967 ::SetSecurityInfo(vhdHandle.get(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, originalOwner, nullptr, nullptr, nullptr));
968 };
test/windows/UnitTests.cpp
+73
@@ -3048,6 +3048,79 @@ Error code: Wsl/InstallDistro/WSL_E_DISTRO_NOT_FOUND
3048 }
3049 }
3050
3051 + WSL2_TEST_METHOD(MoveVhdWithAdminOwner)
3052 + {
3053 + // Regression test for #40716: if the VHD's owner is BUILTIN\Administrators
3054 + // (as happens after a cross-volume MoveFileEx from an elevated context),
3055 + // the move must still succeed because setVhdOwner runs as SYSTEM.
3056 + constexpr auto name = L"move-admin-owner-test-distro";
3057 + constexpr auto firstFolder = L"move-admin-owner-first";
3058 + constexpr auto secondFolder = L"move-admin-owner-second";
3059 +
3060 + // Import a WSL2 distro.
3061 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--import {} . \"{}\" --version 2", name, g_testDistroPath)), 0L);
3062 +
3063 + auto cleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [name]() {
3064 + LxsstuLaunchWsl(std::format(L"--unregister {}", name));
3065 + std::filesystem::remove_all(firstFolder);
3066 + std::filesystem::remove_all(secondFolder);
3067 + });
3068 +
3069 + // Move to first folder so we know where the VHD is.
3070 + WslShutdown();
3071 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, firstFolder)), 0L);
3072 +
3073 + auto vhdPath = std::format(L"{}\\ext4.vhdx", firstFolder);
3074 + VERIFY_IS_TRUE(std::filesystem::exists(vhdPath));
3075 +
3076 + // Simulate cross-volume MoveFileEx side-effect: change VHD owner to BUILTIN\Administrators.
3077 + {
3078 + BYTE adminsSidBuffer[SECURITY_MAX_SID_SIZE];
3079 + DWORD sidSize = sizeof(adminsSidBuffer);
3080 + THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsSidBuffer, &sidSize));
3081 +
3082 + THROW_IF_WIN32_ERROR(SetNamedSecurityInfoW(
3083 + const_cast<LPWSTR>(vhdPath.c_str()), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, adminsSidBuffer, nullptr, nullptr, nullptr));
3084 +
3085 + // Verify it took effect.
3086 + PSID ownerSid = nullptr;
3087 + wil::unique_hlocal descriptor;
3088 + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW(
3089 + vhdPath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor));
3090 + VERIFY_IS_TRUE(EqualSid(ownerSid, adminsSidBuffer));
3091 + }
3092 +
3093 + // Now move again as non-elevated. Before the fix, this would fail with E_ACCESSDENIED
3094 + // because CreateFileW(WRITE_OWNER) was called under user impersonation.
3095 + const auto nonElevatedToken = GetNonElevatedToken();
3096 + WslShutdown();
3097 + VERIFY_ARE_EQUAL(
3098 + LxsstuLaunchWsl(std::format(L"--manage {} --move {}", name, secondFolder), nullptr, nullptr, nullptr, nonElevatedToken.get()), 0L);
3099 +
3100 + auto newVhdPath = std::format(L"{}\\ext4.vhdx", secondFolder);
3101 + VERIFY_IS_TRUE(std::filesystem::exists(newVhdPath));
3102 +
3103 + // Verify the VHD owner was preserved. The code reads the owner before
3104 + // MoveFileEx and restores it afterward. Since we set the owner to
3105 + // Administrators before this move, it should still be Administrators.
3106 + {
3107 + PSID ownerSid = nullptr;
3108 + wil::unique_hlocal descriptor;
3109 + THROW_IF_WIN32_ERROR(GetNamedSecurityInfoW(
3110 + newVhdPath.c_str(), SE_FILE_OBJECT, OWNER_SECURITY_INFORMATION, &ownerSid, nullptr, nullptr, nullptr, &descriptor));
3111 +
3112 + BYTE adminsSidCheck[SECURITY_MAX_SID_SIZE] = {};
3113 + DWORD sidSize = sizeof(adminsSidCheck);
3114 + THROW_IF_WIN32_BOOL_FALSE(CreateWellKnownSid(WinBuiltinAdministratorsSid, nullptr, adminsSidCheck, &sidSize));
3115 + VERIFY_IS_TRUE(EqualSid(ownerSid, adminsSidCheck));
3116 + }
3117 +
3118 + // Validate distro still works.
3119 + WslShutdown();
3120 + auto [out, err] = LxsstuLaunchWslAndCaptureOutput(std::format(L"-d {} echo ok", name), 0, nullptr, nonElevatedToken.get());
3121 + VERIFY_ARE_EQUAL(out, L"ok\n");
3122 + }
3123 +
3124 WSL2_TEST_METHOD(Resize)
3125 {
3126 constexpr auto name = L"resize-test-distro";