microsoft/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2025-12-10 18:43:54 -06:00
Code Issues Packages Projects Releases 138 Wiki Activity

Fix WSL PATH corruption & potential use-after-free (#19211)

Browse Source 
Closes #19152

## Validation Steps Performed
* Set `PATH` on a linux profile
* `PATH` isn't messed up inside WSL ✅

(cherry picked from commit 0a6394270e2026bbd2bf5f937870eb9242f326ed)
Service-Card-Id: PVTI_lADOAF3p4s4AxadtzgdR6bA
Service-Version: 1.23
This commit is contained in:
Leonard Hecker 2025-08-05 00:42:26 +02:00 committed by Dustin L. Howett
parent 4995af3dc1
commit 34cd29a0ce
2 changed files with 21 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/cascadia/TerminalConnection/ConptyConnection.cpp
View File

@ -30,8 +30,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
{ {
// Function Description: // Function Description:
// - launches the client application attached to the new pseudoconsole // - launches the client application attached to the new pseudoconsole
HRESULT ConptyConnection::_LaunchAttachedClient() noexcept void ConptyConnection::_LaunchAttachedClient()
try
{ {
STARTUPINFOEX siEx{ 0 }; STARTUPINFOEX siEx{ 0 };
siEx.StartupInfo.cb = sizeof(STARTUPINFOEX); siEx.StartupInfo.cb = sizeof(STARTUPINFOEX);
@ -43,15 +42,16 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
auto attrList{ std::make_unique<std::byte[]>(size) }; auto attrList{ std::make_unique<std::byte[]>(size) };
#pragma warning(suppress : 26490) // We have to use reinterpret_cast because we allocated a byte array as a proxy for the adjustable size list. #pragma warning(suppress : 26490) // We have to use reinterpret_cast because we allocated a byte array as a proxy for the adjustable size list.
siEx.lpAttributeList = reinterpret_cast<PPROC_THREAD_ATTRIBUTE_LIST>(attrList.get()); siEx.lpAttributeList = reinterpret_cast<PPROC_THREAD_ATTRIBUTE_LIST>(attrList.get());
RETURN_IF_WIN32_BOOL_FALSE(InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, &size)); THROW_IF_WIN32_BOOL_FALSE(InitializeProcThreadAttributeList(siEx.lpAttributeList, 1, 0, &size));
RETURN_IF_WIN32_BOOL_FALSE(UpdateProcThreadAttribute(siEx.lpAttributeList, THROW_IF_WIN32_BOOL_FALSE(UpdateProcThreadAttribute(
0, siEx.lpAttributeList,
PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, 0,
_hPC.get(), PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE,
sizeof(HPCON), _hPC.get(),
nullptr, sizeof(HPCON),
nullptr)); nullptr,
nullptr));
auto cmdline{ wil::ExpandEnvironmentStringsW<std::wstring>(_commandline.c_str()) }; // mutable copy -- required for CreateProcessW auto cmdline{ wil::ExpandEnvironmentStringsW<std::wstring>(_commandline.c_str()) }; // mutable copy -- required for CreateProcessW
auto environment = _initialEnv; auto environment = _initialEnv;
@ -72,7 +72,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
std::wstring additionalWslEnv; std::wstring additionalWslEnv;
// WSLENV.2: Figure out what variables are already in WSLENV. // WSLENV.2: Figure out what variables are already in WSLENV.
std::unordered_set<std::wstring_view> wslEnvVars; std::unordered_set<std::wstring> wslEnvVars{
// We never want to put a custom Windows PATH variable into WSLENV,
// because that would override WSL's computation of the NIX PATH.
L"PATH",
};
for (const auto& part : til::split_iterator{ std::wstring_view{ wslEnv }, L':' }) for (const auto& part : til::split_iterator{ std::wstring_view{ wslEnv }, L':' })
{ {
// Each part may contain a variable name and flags (e.g., /p, /l, etc.) // Each part may contain a variable name and flags (e.g., /p, /l, etc.)
@ -97,25 +101,19 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
} }
} }
// add additional env vars
if (_environment) if (_environment)
{ {
// Order the environment variable names so that resolution order is consistent
// NOTE(lhecker): I'm like 99% sure that this is unnecessary.
std::set<std::wstring, til::env_key_sorter> keys{};
for (const auto item : _environment) for (const auto item : _environment)
{
keys.insert(std::wstring{ item.Key() });
}
// add additional env vars
for (const auto& key : keys)
{ {
try try
{ {
const auto key = item.Key();
// This will throw if the value isn't a string. If that // This will throw if the value isn't a string. If that
// happens, then just skip this entry. // happens, then just skip this entry.
const auto value = winrt::unbox_value<hstring>(_environment.Lookup(key)); const auto value = winrt::unbox_value<hstring>(_environment.Lookup(key));
environment.set_user_environment_var(key.c_str(), value.c_str()); environment.set_user_environment_var(key, value);
// WSLENV.4: Add custom user environment variables to WSLENV. // WSLENV.4: Add custom user environment variables to WSLENV.
if (wslEnvVars.emplace(key).second) if (wslEnvVars.emplace(key).second)
@ -166,7 +164,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
auto [newCommandLine, newStartingDirectory] = Utils::MangleStartingDirectoryForWSL(cmdline, _startingDirectory); auto [newCommandLine, newStartingDirectory] = Utils::MangleStartingDirectoryForWSL(cmdline, _startingDirectory);
const auto startingDirectory = newStartingDirectory.size() > 0 ? newStartingDirectory.c_str() : nullptr; const auto startingDirectory = newStartingDirectory.size() > 0 ? newStartingDirectory.c_str() : nullptr;
RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW( THROW_IF_WIN32_BOOL_FALSE(CreateProcessW(
nullptr, nullptr,
newCommandLine.data(), newCommandLine.data(),
nullptr, // lpProcessAttributes nullptr, // lpProcessAttributes
@ -193,10 +191,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"), TraceLoggingWideString(_clientName.c_str(), "Client", "The attached client process"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
return S_OK;
} }
CATCH_RETURN();
// Who decided that? // Who decided that?
#pragma warning(suppress : 26455) // Default constructor should not throw. Declare it 'noexcept' (f.6). #pragma warning(suppress : 26455) // Default constructor should not throw. Declare it 'noexcept' (f.6).
@ -418,7 +413,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
THROW_IF_FAILED(ConptyShowHidePseudoConsole(_hPC.get(), _initialVisibility)); THROW_IF_FAILED(ConptyShowHidePseudoConsole(_hPC.get(), _initialVisibility));
} }
THROW_IF_FAILED(_LaunchAttachedClient()); _LaunchAttachedClient();
} }
// But if it was an inbound handoff... attempt to synchronize the size of it with what our connection // But if it was an inbound handoff... attempt to synchronize the size of it with what our connection
// window is expecting it to be on the first layout. // window is expecting it to be on the first layout.

2
src/cascadia/TerminalConnection/ConptyConnection.h
View File

@ -58,7 +58,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
static HRESULT NewHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) noexcept; static HRESULT NewHandoff(HANDLE* in, HANDLE* out, HANDLE signal, HANDLE reference, HANDLE server, HANDLE client, const TERMINAL_STARTUP_INFO* startupInfo) noexcept;
static winrt::hstring _commandlineFromProcess(HANDLE process); static winrt::hstring _commandlineFromProcess(HANDLE process);
HRESULT _LaunchAttachedClient() noexcept; void _LaunchAttachedClient();
void _indicateExitWithStatus(unsigned int status) noexcept; void _indicateExitWithStatus(unsigned int status) noexcept;
static std::wstring _formatStatus(uint32_t status); static std::wstring _formatStatus(uint32_t status);
void _LastConPtyClientDisconnected() noexcept; void _LastConPtyClientDisconnected() noexcept;