Skip to content

Commit 1b4a591

Browse files
authored
fix(win): remove MAX_PATH dependency (#135)
* fix(win): remove MAX_PATH dependency * add wchar alignment validation on the server IPC side
1 parent 79a91fe commit 1b4a591

File tree

6 files changed

+220
-21
lines changed

6 files changed

+220
-21
lines changed

client/crashpad_client_win.cc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,21 +1205,19 @@ void CrashpadClient::SetFirstChanceExceptionHandler(
12051205
}
12061206

12071207
void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
1208-
ClientToServerMessage message = {};
1209-
message.type = ClientToServerMessage::kAddAttachment;
1210-
swprintf_s(
1211-
message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str());
12121208
ServerToClientMessage response = {};
1213-
SendToCrashHandlerServer(ipc_pipe_, message, &response);
1209+
SendAttachmentToCrashHandlerServer(ipc_pipe_,
1210+
ClientToServerMessage::kAddAttachmentV2,
1211+
attachment.value(),
1212+
&response);
12141213
}
12151214

12161215
void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
1217-
ClientToServerMessage message = {};
1218-
message.type = ClientToServerMessage::kRemoveAttachment;
1219-
swprintf_s(
1220-
message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str());
12211216
ServerToClientMessage response = {};
1222-
SendToCrashHandlerServer(ipc_pipe_, message, &response);
1217+
SendAttachmentToCrashHandlerServer(ipc_pipe_,
1218+
ClientToServerMessage::kRemoveAttachmentV2,
1219+
attachment.value(),
1220+
&response);
12231221
}
12241222

12251223
} // namespace crashpad

util/misc/paths_win.cc

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,25 @@ namespace crashpad {
2424

2525
// static
2626
bool Paths::Executable(base::FilePath* path) {
27-
wchar_t executable_path[_MAX_PATH];
28-
unsigned int len = GetModuleFileName(
29-
nullptr, executable_path, static_cast<DWORD>(std::size(executable_path)));
27+
// follow the maximum path length documented here:
28+
// https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
29+
constexpr DWORD kMaxPathChars = 32768;
30+
std::wstring executable_path(kMaxPathChars, L'\0');
31+
32+
const DWORD len =
33+
GetModuleFileName(nullptr, &executable_path[0], kMaxPathChars);
34+
3035
if (len == 0) {
3136
PLOG(ERROR) << "GetModuleFileName";
3237
return false;
33-
} else if (len >= std::size(executable_path)) {
34-
LOG(ERROR) << "GetModuleFileName";
38+
}
39+
40+
if (len >= kMaxPathChars) {
41+
LOG(ERROR) << "GetModuleFileName: path exceeds maximum length";
3542
return false;
3643
}
3744

45+
executable_path.resize(len);
3846
*path = base::FilePath(executable_path);
3947
return true;
4048
}

util/win/exception_handler_server.cc

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,14 @@ void ExceptionHandlerServer::InitializeWithInheritedDataForInitialClient(
295295

296296
first_pipe_instance_.reset(initial_client_data.first_pipe_instance());
297297

298-
// TODO(scottmg): Vista+. Might need to pass through or possibly find an Nt*.
299-
auto data = base::HeapArray<uint8_t>::Uninit(sizeof(wchar_t) * _MAX_PATH +
300-
sizeof(FILE_NAME_INFO));
298+
// Allocate buffer for FILE_NAME_INFO with maximum pipe name length.
299+
// According to Windows documentation, pipe name strings are limited to 256
300+
// characters: https://learn.microsoft.com/en-us/windows/win32/ipc/pipe-names
301+
// FILE_NAME_INFO has a flexible array member (FileName) and provides an
302+
// explicit FileNameLength field, so no null terminator is needed.
303+
constexpr size_t kMaxPipeNameChars = 256;
304+
auto data = base::HeapArray<uint8_t>::Uninit(
305+
sizeof(FILE_NAME_INFO) + sizeof(wchar_t) * kMaxPipeNameChars);
301306
if (!GetFileInformationByHandleEx(first_pipe_instance_.get(),
302307
FileNameInfo,
303308
data.data(),
@@ -414,6 +419,70 @@ void ExceptionHandlerServer::Stop() {
414419
PostQueuedCompletionStatus(port_.get(), 0, 0, nullptr);
415420
}
416421

422+
static void HandleAddAttachmentV2(
423+
const internal::PipeServiceContext& service_context,
424+
const ClientToServerMessage& message) {
425+
const uint32_t path_length_bytes = message.attachment_v2.path_length_bytes;
426+
427+
if (path_length_bytes == 0 || path_length_bytes > kMaxPathBytes) {
428+
LOG(ERROR) << "Invalid path length: " << path_length_bytes;
429+
return;
430+
}
431+
432+
if (path_length_bytes % sizeof(wchar_t) != 0) {
433+
LOG(ERROR) << "Invalid path length: not aligned to wchar_t boundary";
434+
return;
435+
}
436+
437+
auto path_buffer =
438+
base::HeapArray<wchar_t>::Uninit(path_length_bytes / sizeof(wchar_t));
439+
440+
if (!LoggingReadFileExactly(
441+
service_context.pipe(), path_buffer.data(), path_length_bytes)) {
442+
LOG(ERROR) << "Failed to read attachment path";
443+
return;
444+
}
445+
446+
path_buffer[path_buffer.size() - 1] = L'\0';
447+
448+
ServerToClientMessage response = {};
449+
service_context.delegate()->ExceptionHandlerServerAttachmentAdded(
450+
base::FilePath(std::wstring(path_buffer.data())));
451+
LoggingWriteFile(service_context.pipe(), &response, sizeof(response));
452+
}
453+
454+
static void HandleRemoveAttachmentV2(
455+
const internal::PipeServiceContext& service_context,
456+
const ClientToServerMessage& message) {
457+
const uint32_t path_length_bytes = message.attachment_v2.path_length_bytes;
458+
459+
if (path_length_bytes == 0 || path_length_bytes > kMaxPathBytes) {
460+
LOG(ERROR) << "Invalid path length: " << path_length_bytes;
461+
return;
462+
}
463+
464+
if (path_length_bytes % sizeof(wchar_t) != 0) {
465+
LOG(ERROR) << "Invalid path length: not aligned to wchar_t boundary";
466+
return;
467+
}
468+
469+
auto path_buffer =
470+
base::HeapArray<wchar_t>::Uninit(path_length_bytes / sizeof(wchar_t));
471+
472+
if (!LoggingReadFileExactly(
473+
service_context.pipe(), path_buffer.data(), path_length_bytes)) {
474+
LOG(ERROR) << "Failed to read attachment path";
475+
return;
476+
}
477+
478+
path_buffer[path_buffer.size() - 1] = L'\0';
479+
480+
ServerToClientMessage response = {};
481+
service_context.delegate()->ExceptionHandlerServerAttachmentRemoved(
482+
base::FilePath(std::wstring(path_buffer.data())));
483+
LoggingWriteFile(service_context.pipe(), &response, sizeof(response));
484+
}
485+
417486
// This function must be called with service_context.pipe() already connected to
418487
// a client pipe. It exchanges data with the client and adds a ClientData record
419488
// to service_context->clients().
@@ -475,6 +544,16 @@ bool ExceptionHandlerServer::ServiceClientConnection(
475544
return false;
476545
}
477546

547+
case ClientToServerMessage::kAddAttachmentV2: {
548+
HandleAddAttachmentV2(service_context, message);
549+
return false;
550+
}
551+
552+
case ClientToServerMessage::kRemoveAttachmentV2: {
553+
HandleRemoveAttachmentV2(service_context, message);
554+
return false;
555+
}
556+
478557
default:
479558
LOG(ERROR) << "unhandled message type: " << message.type;
480559
return false;

util/win/registration_protocol_win.cc

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,81 @@ bool SendToCrashHandlerServer(const std::wstring& pipe_name,
141141
}
142142
}
143143

144+
bool SendAttachmentToCrashHandlerServer(
145+
const std::wstring& pipe_name,
146+
ClientToServerMessage::Type message_type,
147+
const std::wstring& path,
148+
ServerToClientMessage* response) {
149+
if (message_type != ClientToServerMessage::kAddAttachmentV2 &&
150+
message_type != ClientToServerMessage::kRemoveAttachmentV2) {
151+
LOG(ERROR) << "Invalid message type for attachment: " << message_type;
152+
return false;
153+
}
154+
155+
const size_t path_length_bytes = (path.length() + 1) * sizeof(wchar_t);
156+
157+
if (path_length_bytes > kMaxPathBytes) {
158+
LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes";
159+
return false;
160+
}
161+
162+
// Build the message header.
163+
ClientToServerMessage message = {};
164+
message.type = message_type;
165+
message.attachment_v2.path_length_bytes =
166+
static_cast<uint32_t>(path_length_bytes);
167+
168+
// Retry CreateFile() in a loop (follows the logic in
169+
// SendToCrashHandlerServer).
170+
for (;;) {
171+
ScopedFileHANDLE pipe(
172+
CreateFile(pipe_name.c_str(),
173+
GENERIC_READ | GENERIC_WRITE,
174+
0,
175+
nullptr,
176+
OPEN_EXISTING,
177+
SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION,
178+
nullptr));
179+
if (!pipe.is_valid()) {
180+
if (GetLastError() != ERROR_PIPE_BUSY) {
181+
PLOG(ERROR) << "CreateFile";
182+
return false;
183+
}
184+
185+
if (!WaitNamedPipe(pipe_name.c_str(), NMPWAIT_WAIT_FOREVER)) {
186+
PLOG(ERROR) << "WaitNamedPipe";
187+
return false;
188+
}
189+
190+
continue;
191+
}
192+
193+
DWORD mode = PIPE_READMODE_MESSAGE;
194+
if (!SetNamedPipeHandleState(pipe.get(), &mode, nullptr, nullptr)) {
195+
PLOG(ERROR) << "SetNamedPipeHandleState";
196+
return false;
197+
}
198+
199+
if (!WriteFile(pipe.get(), &message, sizeof(message))) {
200+
PLOG(ERROR) << "WriteFile (header)";
201+
return false;
202+
}
203+
204+
if (!WriteFile(
205+
pipe.get(), path.c_str(), static_cast<DWORD>(path_length_bytes))) {
206+
PLOG(ERROR) << "WriteFile (path)";
207+
return false;
208+
}
209+
210+
if (!ReadFile(pipe.get(), response, sizeof(*response))) {
211+
PLOG(ERROR) << "ReadFile (response)";
212+
return false;
213+
}
214+
215+
return true;
216+
}
217+
}
218+
144219
HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
145220
bool first_instance) {
146221
SECURITY_ATTRIBUTES security_attributes;

util/win/registration_protocol_win.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,24 @@ bool SendToCrashHandlerServer(const std::wstring& pipe_name,
3636
const ClientToServerMessage& message,
3737
ServerToClientMessage* response);
3838

39+
//! \brief Connect over the given \a pipe_name, passing a variable-length
40+
//! attachment message to the server.
41+
//!
42+
//! This is used for kAddAttachmentV2 and kRemoveAttachmentV2 message types
43+
//! which support paths longer than MAX_PATH.
44+
//!
45+
//! \param[in] pipe_name The name of the pipe to connect to.
46+
//! \param[in] message_type Either kAddAttachmentV2 or kRemoveAttachmentV2.
47+
//! \param[in] path The attachment path to send.
48+
//! \param[out] response The server's response.
49+
//!
50+
//! \return `true` on success, `false` on failure with a message logged.
51+
bool SendAttachmentToCrashHandlerServer(
52+
const std::wstring& pipe_name,
53+
ClientToServerMessage::Type message_type,
54+
const std::wstring& path,
55+
ServerToClientMessage* response);
56+
3957
//! \brief Wraps CreateNamedPipe() to create a single named pipe instance.
4058
//!
4159
//! \param[in] pipe_name The name to use for the pipe.

util/win/registration_protocol_win_structs.h

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,26 @@ struct ShutdownRequest {
116116
uint64_t token;
117117
};
118118

119-
//! \brief A file attachment request.
119+
//! \brief A file attachment request (legacy, limited to MAX_PATH).
120120
struct AttachmentRequest {
121121
//! \brief The path of the attachment.
122122
wchar_t path[MAX_PATH];
123123
};
124124

125+
//! \brief A variable-length file attachment request header.
126+
//!
127+
//! For kAddAttachmentV2 and kRemoveAttachmentV2, the message consists of
128+
//! a ClientToServerMessage with this header in the union, followed by
129+
//! path_length_bytes of wchar_t data containing the null-terminated path.
130+
struct AttachmentRequestV2 {
131+
//! \brief Length of the path in bytes, including null terminator.
132+
uint32_t path_length_bytes;
133+
};
134+
135+
//! follow the maximum path length documented here:
136+
//! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation
137+
constexpr uint32_t kMaxPathBytes = 32768 * sizeof(wchar_t);
138+
125139
//! \brief The message passed from client to server by
126140
//! SendToCrashHandlerServer().
127141
struct ClientToServerMessage {
@@ -133,22 +147,29 @@ struct ClientToServerMessage {
133147
//! \brief For ShutdownRequest.
134148
kShutdown,
135149

136-
//! \brief For AttachmentRequest.
150+
//! \brief For AttachmentRequest (legacy, MAX_PATH limited).
137151
kAddAttachment,
138152

139-
//! \brief For AttachmentRequest.
153+
//! \brief For AttachmentRequest (legacy, MAX_PATH limited).
140154
kRemoveAttachment,
141155

142156
//! \brief An empty message sent by the initial client in asynchronous mode.
143157
//! No data is required, this just confirms that the server is ready to
144158
//! accept client registrations.
145159
kPing,
160+
161+
//! \brief For AttachmentRequestV2 (variable-length paths).
162+
kAddAttachmentV2,
163+
164+
//! \brief For AttachmentRequestV2 (variable-length paths).
165+
kRemoveAttachmentV2,
146166
} type;
147167

148168
union {
149169
RegistrationRequest registration;
150170
ShutdownRequest shutdown;
151171
AttachmentRequest attachment;
172+
AttachmentRequestV2 attachment_v2;
152173
};
153174
};
154175

0 commit comments

Comments
 (0)