Bound process name buffer in crash dump handler (#40274)
CollectCrashDumps cast the LX_PROCESS_CRASH flexible array member Buffer to const char* without verifying NUL-termination. Use the response span from Receive() to compute the exact buffer size, then construct a bounded std::string via strnlen. Also fix undefined behavior in std::isalnum with negative char values. Co-authored-by: Ben Hillis <benhill@ntdev.microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ben Hillis committed
Apr 22, 2026 at 15:24 UTC
901db6de0c987a9b01665ec09d3523067a39b7b9
1 file changed
+13
-4
src/windows/service/exe/WslCoreVm.cpp
+13
-4
@@ -1100,15 +1100,24 @@ void WslCoreVm::CollectCrashDumps(wil::unique_socket&& listenSocket) const
1100
auto channel = wsl::shared::SocketChannel{std::move(socket.value()), "crash_dump", m_terminatingEvent.get()};
1101
1102
auto transaction = channel.ReceiveTransaction();
1103
- const auto& message = transaction.Receive<LX_PROCESS_CRASH>();
1104
- const char* process = reinterpret_cast<const char*>(&message.Buffer);
1103
+ gsl::span<gsl::byte> responseSpan;
1104
+ const auto& message = transaction.Receive<LX_PROCESS_CRASH>(&responseSpan);
1105
+
1106
+ // Safely extract the process name from the flexible array member.
1107
+ // The buffer may not be NUL-terminated, so bound the length to the received span size.
1108
+ const auto bufferSize = responseSpan.size_bytes() - offsetof(LX_PROCESS_CRASH, Buffer);
1109
+ const std::string process(message.Buffer, strnlen(message.Buffer, bufferSize));
1110
1111
constexpr auto dumpExtension = ".dmp";
1112
constexpr auto dumpPrefix = "wsl-crash";
1113
1114
auto filename = std::format("{}-{}-{}-{}-{}{}", dumpPrefix, message.Timestamp, message.Pid, process, message.Signal, dumpExtension);
1115
1111
- std::replace_if(filename.begin(), filename.end(), [](auto e) { return !std::isalnum(e) && e != '.' && e != '-'; }, '_');
1116
+ std::replace_if(
1117
+ filename.begin(),
1118
+ filename.end(),
1119
+ [](char e) { return !std::isalnum(static_cast<unsigned char>(e)) && e != '.' && e != '-'; },
1120
+ '_');
1121
1122
auto fullPath = m_vmConfig.CrashDumpFolder / filename;
1123
@@ -1119,7 +1128,7 @@ void WslCoreVm::CollectCrashDumps(wil::unique_socket&& listenSocket) const
1128
TraceLoggingValue(fullPath.c_str(), "FullPath"),
1129
TraceLoggingValue(message.Pid, "Pid"),
1130
TraceLoggingValue(message.Signal, "Signal"),
1122
- TraceLoggingValue(process, "process"));
1131
+ TraceLoggingValue(process.c_str(), "process"));
1132
1133
auto runAsUser = wil::impersonate_token(m_userToken.get());
1134