@samitouri / QOSAMI-WSL-1 / commits / f0f4b106

Fix disk-attach restore TOCTOU by impersonating the mounting user on VHD restore (#40782)

* Fix disk-attach restore TOCTOU by impersonating the mounting user on VHD restore A standard user's live ' wsl --mount --vhd' is already safe from a junction/symlink swap: the VM access grant runs while impersonating the user, and the SYSTEM-side AddVhd only succeeds on a file the VM was granted access to, so a swap yields ACCESS_DENIED rather than disclosure. The actual gap was disk restore: when the VM is recreated, _LoadDiskMount re-attached persisted VHDs as SYSTEM (no token), re-resolving a user-controllable path and reopening the TOCTOU. Because the disk-mount state is stored under the user's SID in a volatile (per-boot) key, the disk being restored was mounted by this same user in this same boot, so we can simply pass the user token and let the existing impersonated grant close the window. Pass-through devices stay SYSTEM (elevation-gated; \\.\PhysicalDriveN has no reparse surface). This replaces the earlier handle-pinning/reparse-rejection approach, which also regressed legitimate symlinked VHDs. Add tests covering a symlinked VHD mounting and surviving a VM idle-timeout restore. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR feedback: re-query block device after VM timeout and require symlink creation in mount tests 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 23, 2026 at 14:22 UTC f0f4b10697d0496fa61a71a05447ecb3fefa015c
2 files changed +72 -3
src/windows/service/exe/LxssUserSession.cpp
+13 -3
@@ -3210,10 +3210,20 @@ try
3210
3211 // Attach the disk to the VM, reusing the same LUN if possible.
3212 //
3213 - // N.B. The user token is not provided because the key that holds the disk
3214 - // state can only be written by elevated users.
3213 + // N.B. The disk-mount state is stored under the user's SID in a volatile (per-boot)
3214 + // registry key, so the disk being restored here was mounted earlier in this same boot
3215 + // by this same user. For a VHD we therefore pass the user token so the access grant and
3216 + // the path resolution run under the mounting user's identity: a privileged operation can
3217 + // only ever touch a file that user can already reach, which closes the restore-time
3218 + // junction/symlink swap (TOCTOU) without re-resolving the path as SYSTEM.
3219 + //
3220 + // A pass-through (raw block device) attach is elevation-gated and the reconnecting user
3221 + // may no longer be elevated, so it is restored as SYSTEM (no token). Block-device paths
3222 + // (\\.\PhysicalDriveN) have no reparse-point surface, so there is no swap to defend
3223 + // against.
3224 auto lun = std::stoul(LunStr);
3216 - m_utilityVm->AttachDisk(path.c_str(), diskType, lun, true, nullptr);
3225 + const HANDLE userToken = (diskType == WslCoreVm::DiskType::VHD) ? m_userToken.get() : nullptr;
3226 + m_utilityVm->AttachDisk(path.c_str(), diskType, lun, true, userToken);
3227
3228 // Restore each mount point.
3229 for (const auto& e : wsl::windows::common::registry::EnumKeys(Key, KEY_READ))
test/windows/MountTests.cpp
+59
@@ -460,6 +460,65 @@ class MountTests
460 VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--unmount " + absolutePath.wstring()), (DWORD)0);
461 }
462
463 + // A VHD whose path is a symbolic link is a legitimate, supported scenario: the link is
464 + // followed and the real VHD is attached. Access is granted while impersonating the user,
465 + // so the user can only ever attach a file they can already reach; there is no need to
466 + // reject reparse points in the path.
467 + WSL2_TEST_METHOD(MountVhdThroughSymlinkSucceeds)
468 + {
469 + SKIP_UNSUPPORTED_ARM64_MOUNT_TEST();
470 +
471 + const auto symlink = std::filesystem::absolute(L"TestVhdSymlink.vhd");
472 + DeleteFileW(symlink.c_str());
473 +
474 + const auto absoluteTarget = std::filesystem::absolute(TEST_MOUNT_VHD);
475 +
476 + // Create a file symbolic link pointing at the real VHD.
477 + VERIFY_IS_TRUE(CreateSymbolicLinkW(symlink.c_str(), absoluteTarget.c_str(), SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE));
478 +
479 + auto cleanup = wil::scope_exit([&]() { DeleteFileW(symlink.c_str()); });
480 +
481 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--mount " + symlink.wstring() + L" --vhd --bare"), (DWORD)0);
482 +
483 + const auto disk = GetBlockDeviceInWsl();
484 + VERIFY_IS_TRUE(IsBlockDevicePresent(disk));
485 +
486 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--unmount " + symlink.wstring()), (DWORD)0);
487 + }
488 +
489 + // A symlinked VHD must still be restored after the VM is torn down on idle. Restore runs
490 + // under the mounting user's identity (the disk-mount state is stored per-SID for the
491 + // current boot), so the same access check applies and the symlinked VHD re-attaches.
492 + WSL2_TEST_METHOD(MountVhdThroughSymlinkSurvivesVmTimeout)
493 + {
494 + SKIP_UNSUPPORTED_ARM64_MOUNT_TEST();
495 +
496 + const auto symlink = std::filesystem::absolute(L"TestVhdSymlinkRestore.vhd");
497 + DeleteFileW(symlink.c_str());
498 +
499 + const auto absoluteTarget = std::filesystem::absolute(TEST_MOUNT_VHD);
500 +
501 + VERIFY_IS_TRUE(CreateSymbolicLinkW(symlink.c_str(), absoluteTarget.c_str(), SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE));
502 +
503 + auto cleanup = wil::scope_exit([&]() { DeleteFileW(symlink.c_str()); });
504 +
505 + WslKeepAlive keepAlive;
506 +
507 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--mount " + symlink.wstring() + L" --vhd --bare"), (DWORD)0);
508 +
509 + auto disk = GetBlockDeviceInWsl();
510 + VERIFY_IS_TRUE(IsBlockDevicePresent(disk));
511 +
512 + WaitForVmTimeout(keepAlive);
513 +
514 + // Recreating the VM restores the persisted disk mount; the symlinked VHD must re-attach. The
515 + // block device name is not guaranteed to be stable across the VM teardown, so re-query it.
516 + disk = GetBlockDeviceInWsl();
517 + VERIFY_IS_TRUE(IsBlockDevicePresent(disk));
518 +
519 + VERIFY_ARE_EQUAL(LxsstuLaunchWsl(L"--unmount " + symlink.wstring()), (DWORD)0);
520 + }
521 +
522 // Attach a disk, but don't mount it
523 WSL2_TEST_METHOD(TestBareMount)
524 {