From 94242690d57be5df0dd60cc51bc2f41bb451ff89 Mon Sep 17 00:00:00 2001 From: Justin Cohen Date: Tue, 10 May 2022 10:41:04 -0400 Subject: [PATCH] ios: Check dyld_image_info->imageFilePath for nullptr. It seems on iOS 14, sometimes this path can be empty. Passing nullptr to strlen will crash. Also fixes an incorrect file path length for the dyldPath. Bug: 1323905 Change-Id: Idf1ef9e0165853a5d57d272896a40bf0b30a3368 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3637717 Reviewed-by: Joshua Peraza Commit-Queue: Justin Cohen --- .../in_process_intermediate_dump_handler.cc | 17 +++++++--- ...ess_snapshot_ios_intermediate_dump_test.cc | 33 +++++++++++-------- 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/client/ios_handler/in_process_intermediate_dump_handler.cc b/client/ios_handler/in_process_intermediate_dump_handler.cc index 88d5eb0a..2869c217 100644 --- a/client/ios_handler/in_process_intermediate_dump_handler.cc +++ b/client/ios_handler/in_process_intermediate_dump_handler.cc @@ -963,10 +963,12 @@ void InProcessIntermediateDumpHandler::WriteModuleInfo( return; } - WriteProperty(writer, - IntermediateDumpKey::kName, - image->imageFilePath, - strlen(image->imageFilePath)); + if (image->imageFilePath) { + WriteProperty(writer, + IntermediateDumpKey::kName, + image->imageFilePath, + strlen(image->imageFilePath)); + } uint64_t address = FromPointerCast(image->imageLoadAddress); WriteProperty(writer, IntermediateDumpKey::kAddress, &address); WriteProperty( @@ -976,7 +978,12 @@ void InProcessIntermediateDumpHandler::WriteModuleInfo( { IOSIntermediateDumpWriter::ScopedArrayMap modules(writer); - WriteProperty(writer, IntermediateDumpKey::kName, image_infos->dyldPath); + if (image_infos->dyldPath) { + WriteProperty(writer, + IntermediateDumpKey::kName, + image_infos->dyldPath, + strlen(image_infos->dyldPath)); + } uint64_t address = FromPointerCast(image_infos->dyldImageLoadAddress); WriteProperty(writer, IntermediateDumpKey::kAddress, &address); diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc b/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc index 5f56082e..44a7b775 100644 --- a/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc +++ b/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc @@ -198,14 +198,16 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test { } } - void WriteModules(IOSIntermediateDumpWriter* writer) { + void WriteModules(IOSIntermediateDumpWriter* writer, bool has_module_path) { IOSIntermediateDumpWriter::ScopedArray moduleArray(writer, Key::kModules); for (uint32_t image_index = 0; image_index < 2; ++image_index) { IOSIntermediateDumpWriter::ScopedArrayMap modules(writer); - constexpr char image_file[] = "/path/to/module"; - EXPECT_TRUE( - writer->AddProperty(Key::kName, image_file, strlen(image_file))); + if (has_module_path) { + constexpr char image_file[] = "/path/to/module"; + EXPECT_TRUE( + writer->AddProperty(Key::kName, image_file, strlen(image_file))); + } uint64_t address = 0; uint64_t vmsize = 1; @@ -241,12 +243,16 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test { } } - void ExpectModules(const std::vector& modules) { + void ExpectModules(const std::vector& modules, + bool expect_module_path) { for (auto module : modules) { EXPECT_EQ(module->GetModuleType(), ModuleSnapshot::kModuleTypeSharedLibrary); - EXPECT_STREQ(module->Name().c_str(), "/path/to/module"); - EXPECT_STREQ(module->DebugFileName().c_str(), "module"); + + if (expect_module_path) { + EXPECT_STREQ(module->Name().c_str(), "/path/to/module"); + EXPECT_STREQ(module->DebugFileName().c_str(), "module"); + } UUID uuid; uint32_t age; module->UUIDAndAge(&uuid, &age); @@ -424,7 +430,8 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test { EXPECT_STREQ(daylight_name.c_str(), "Daylight"); } - void ExpectSnapshot(const ProcessSnapshot& snapshot) { + void ExpectSnapshot(const ProcessSnapshot& snapshot, + bool expect_module_path) { EXPECT_EQ(snapshot.ProcessID(), 2); EXPECT_EQ(snapshot.ParentProcessID(), 1); @@ -447,7 +454,7 @@ class ProcessSnapshotIOSIntermediateDumpTest : public testing::Test { ExpectSystem(*snapshot.System()); ExpectThreads(snapshot.Threads()); - ExpectModules(snapshot.Modules()); + ExpectModules(snapshot.Modules(), expect_module_path); ExpectMachException(*snapshot.Exception()); } @@ -626,14 +633,14 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, ShortContext) { WriteSystemInfo(writer()); WriteProcessInfo(writer()); WriteThreads(writer()); - WriteModules(writer()); + WriteModules(writer(), /*has_module_path=*/false); WriteMachException(writer(), true /* short_context=true*/); } ProcessSnapshotIOSIntermediateDump process_snapshot; ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); - ExpectSnapshot(process_snapshot); + ExpectSnapshot(process_snapshot, /*expect_module_path=*/false); } TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FullReport) { @@ -644,14 +651,14 @@ TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FullReport) { WriteSystemInfo(writer()); WriteProcessInfo(writer()); WriteThreads(writer()); - WriteModules(writer()); + WriteModules(writer(), /*has_module_path=*/true); WriteMachException(writer()); } ProcessSnapshotIOSIntermediateDump process_snapshot; ASSERT_TRUE(process_snapshot.InitializeWithFilePath(path(), annotations())); EXPECT_FALSE(IsRegularFile(path())); EXPECT_TRUE(DumpSnapshot(process_snapshot)); - ExpectSnapshot(process_snapshot); + ExpectSnapshot(process_snapshot, /*expect_module_path=*/true); } TEST_F(ProcessSnapshotIOSIntermediateDumpTest, FuzzTestCases) {