3party/crashpad
0
0
Fork 0
mirror of https://github.com/chromium/crashpad.git synced 2024-12-27 15:32:10 +08:00
Code Issues Projects Releases Wiki Activity

Fix off-by-one error in ModuleSnapshotMinidump DebugFileName.

Browse Source 
Strings in minidumps are typically NUL-terminated
(https://docs.microsoft.com/en-us/windows/win32/api/minidumpapiset/ns-minidumpapiset-minidump_string).
But the CodeViewRecord types do not use MINDIUMP_STRINGs, and do not
have a separate length field for the pdb_name. Instead, the strings are
always NUL-terminated, with the length derived from the
MINIDUMP_LOCATION_DESCRIPTOR::DataSize field. The writer is correctly
NUL-terminating the debug filename, but ModuleSnapshotMinidump is
off-by-one and including the NUL-terminator.

Change-Id: I8d813b5ef9e9e167dca73a6a938fbbf8dd1580c2
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3482876
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
This commit is contained in:
Justin Cohen 2022-02-24 16:38:28 -05:00 committed by Crashpad LUCI CQ
parent e9937cb36c
commit 2bb6f068a8
2 changed files with 16 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
snapshot/minidump/module_snapshot_minidump.cc
View File

@ -100,7 +100,7 @@ bool ModuleSnapshotMinidump::InitializeModuleCodeView(
signature = *reinterpret_cast<uint32_t*>(cv_record.data());
if (signature == CodeViewRecordPDB70::kSignature) {
if (cv_record.size() < offsetof(CodeViewRecordPDB70, pdb_name)) {
if (cv_record.size() < offsetof(CodeViewRecordPDB70, pdb_name) + 1) {
LOG(ERROR) << "CodeView record in module marked as PDB70 but too small";
return false;
}
@ -111,8 +111,14 @@ bool ModuleSnapshotMinidump::InitializeModuleCodeView(
age_ = cv_record_pdb70->age;
uuid_ = cv_record_pdb70->uuid;
if (cv_record.back() != '\0') {
LOG(ERROR) << "CodeView record marked as PDB70 missing NUL-terminator in "
"pdb_name";
return false;
}
std::copy(cv_record.begin() + offsetof(CodeViewRecordPDB70, pdb_name),
cv_record.end(),
cv_record.end() - 1,
std::back_inserter(debug_file_name_));
return true;
}

11
snapshot/minidump/process_snapshot_minidump_test.cc
View File

@ -350,6 +350,7 @@ TEST(ProcessSnapshotMinidump, Modules) {
"libgeorgism",
"librealistutopia",
};
constexpr char debug_name[] = "debugme.pdb";
minidump_module.BaseOfImage = 0xbadf00d;
minidump_module.SizeOfImage = 9001;
@ -373,12 +374,15 @@ TEST(ProcessSnapshotMinidump, Modules) {
pdb70_cv.signature = CodeViewRecordPDB70::kSignature;
pdb70_cv.age = 7;
pdb70_cv.uuid.InitializeFromString("00112233-4455-6677-8899-aabbccddeeff");
pdb70_cv.pdb_name[0] = '\0';
auto pdb70_loc = static_cast<RVA>(string_file.SeekGet());
auto pdb70_size = sizeof(pdb70_cv);
auto pdb70_size = offsetof(CodeViewRecordPDB70, pdb_name);
EXPECT_TRUE(string_file.Write(&pdb70_cv, sizeof(pdb70_cv)));
EXPECT_TRUE(string_file.Write(&pdb70_cv, pdb70_size));
size_t nul_terminated_length = strlen(debug_name) + 1;
EXPECT_TRUE(string_file.Write(debug_name, nul_terminated_length));
pdb70_size += nul_terminated_length;
CodeViewRecordBuildID build_id_cv;
build_id_cv.signature = CodeViewRecordBuildID::kSignature;
@ -545,6 +549,7 @@ TEST(ProcessSnapshotMinidump, Modules) {
EXPECT_EQ(uuid.ToString(), "00112233-4455-6677-8899-aabbccddeeff");
EXPECT_EQ(age, 7U);
EXPECT_EQ(modules[i]->DebugFileName(), debug_name);
} else {
auto build_id = modules[i]->BuildID();
std::string build_id_text(build_id.data(),