@samitouri / QOSAMI-WSL-2 / commits / e01a6728

Fix 4 code bugs: substr off-by-one, HANDLE* cast, TOCTOU GetLastError, sun_path overflow (#14297)

Bug 1 - LxssHttpProxy.cpp: IPv6 substr extraction used wrong length calculation. substr(openBracket+1, closeBracket-1) is incorrect when openBracket > 0; fixed to substr(openBracket+1, closeBracket-openBracket-1). Also fixed empty-address guard to check closeBracket (not closeBracket-1). Bug 2 - LxssUserSession.cpp: Two instances of reinterpret_cast<HANDLE*> in ScopedMultiRelay construction should be reinterpret_cast<HANDLE> (without the pointer). Other identical callsites in the same file already use the correct cast. Bug 3 - LxssUserSession.cpp: GetLastError() was called unconditionally after CreateFileW, even on success. A stale ERROR_SHARING_VIOLATION from a prior API call could cause a false throw. Fixed to only check GetLastError() when CreateFileW fails (!vhd). Bug 4 - plan9.cpp: sun_path bounds check used > instead of >= leaving no room for null terminator. Also added a post-split check to ensure the child name fits after splitting parent/child for long paths. Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com>

Ben Hillis committed Feb 26, 2026 at 17:03 UTC e01a672880f38abb8717a972ce11f34fa7fefa9e
3 files changed +16 -9
src/linux/init/plan9.cpp
+5 -2
@@ -51,9 +51,9 @@ wil::unique_fd CreateUnixServerSocket(const char* path)
51 }
52 });
53
54 - // Check if the path will fit in a sockaddr_un.
54 + // Check if the path will fit in a sockaddr_un (with room for null terminator).
55 std::string_view pathView{path};
56 - if (pathView.length() > sizeof(sockaddr_un::sun_path))
56 + if (pathView.length() >= sizeof(sockaddr_un::sun_path))
57 {
58 // It won't, so split the parent path and child name.
59 auto index = pathView.find_last_of('/');
@@ -64,6 +64,9 @@ wil::unique_fd CreateUnixServerSocket(const char* path)
64 const std::string parent{pathView.substr(0, index)};
65 pathView = pathView.substr(index + 1);
66
67 + // Ensure the child name fits in sun_path (with null terminator).
68 + THROW_ERRNO_IF(ENAMETOOLONG, pathView.length() >= sizeof(sockaddr_un::sun_path));
69 +
70 // Get the current working directory to restore it later, and change to the socket's parent
71 // path.
72 oldCwd = getcwd(oldCwdBuffer, sizeof(oldCwdBuffer));
src/windows/service/exe/LxssHttpProxy.cpp
+2 -2
@@ -327,12 +327,12 @@ try
327 if (openBracket != ::std::wstring::npos)
328 {
329 const auto closeBracket = portRemoved.find_first_of(L"]");
330 - if (closeBracket == ::std::wstring::npos || (openBracket + 1 >= closeBracket - 1))
330 + if (closeBracket == ::std::wstring::npos || (openBracket + 1 >= closeBracket))
331 {
332 // no other of below checks can contain brackets
333 return UnsupportedProxyReason::Supported;
334 }
335 - portRemoved = portRemoved.substr(openBracket + 1, closeBracket - 1);
335 + portRemoved = portRemoved.substr(openBracket + 1, closeBracket - openBracket - 1);
336 }
337
338 in6_addr addrV6{};
src/windows/service/exe/LxssUserSession.cpp
+9 -5
@@ -1737,11 +1737,15 @@ try
1737 RETURN_HR_IF(WSL_E_DISTRO_NOT_STOPPED, m_runningInstances.contains(*DistroGuid));
1738
1739 const wil::unique_hfile vhd{::CreateFileW(configuration.VhdFilePath.c_str(), GENERIC_WRITE, 0, nullptr, OPEN_EXISTING, 0, nullptr)};
1740 - if (const DWORD err = GetLastError(); err == ERROR_SHARING_VIOLATION)
1740 + if (!vhd)
1741 {
1742 - THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(err), wsl::shared::Localization::MessageVhdInUse());
1742 + const DWORD err = GetLastError();
1743 + if (err == ERROR_SHARING_VIOLATION)
1744 + {
1745 + THROW_HR_WITH_USER_ERROR(HRESULT_FROM_WIN32(err), wsl::shared::Localization::MessageVhdInUse());
1746 + }
1747 + THROW_WIN32(err);
1748 }
1744 - THROW_LAST_ERROR_IF(!vhd);
1749
1750 FILE_SET_SPARSE_BUFFER buffer{
1751 .SetSparse = Sparse,
@@ -1949,7 +1953,7 @@ HRESULT LxssUserSessionImpl::SetVersion(_In_ LPCGUID DistroGuid, _In_ ULONG Vers
1953 auto wsl1Pipe = wsl::windows::common::wslutil::OpenAnonymousPipe(LX_RELAY_BUFFER_SIZE, true, true);
1954
1955 wsl::windows::common::relay::ScopedMultiRelay stdErrRelay(
1952 - std::vector<HANDLE>{wsl1Pipe.first.get(), reinterpret_cast<HANDLE*>(vmContext.errorSocket.get())}, onTarOutput);
1956 + std::vector<HANDLE>{wsl1Pipe.first.get(), reinterpret_cast<HANDLE>(vmContext.errorSocket.get())}, onTarOutput);
1957
1958 // Add mounts for the rootfs and tools.
1959 auto mounts = _CreateSetupMounts(configuration);
@@ -2014,7 +2018,7 @@ HRESULT LxssUserSessionImpl::SetVersion(_In_ LPCGUID DistroGuid, _In_ ULONG Vers
2018 auto wsl1Pipe = wsl::windows::common::wslutil::OpenAnonymousPipe(LX_RELAY_BUFFER_SIZE, true, true);
2019
2020 wsl::windows::common::relay::ScopedMultiRelay stdErrRelay(
2017 - std::vector<HANDLE>{wsl1Pipe.first.get(), reinterpret_cast<HANDLE*>(vmContext.errorSocket.get())}, onTarOutput);
2021 + std::vector<HANDLE>{wsl1Pipe.first.get(), reinterpret_cast<HANDLE>(vmContext.errorSocket.get())}, onTarOutput);
2022
2023 // Add mounts for the rootfs and tools.
2024 auto mounts = _CreateSetupMounts(configuration);