commit 94242690d57be5df0dd60cc51bc2f41bb451ff89
author Justin Cohen <justincohen@google.com> Tue May 10 14:41:04 2022
committer Crashpad LUCI CQ <crashpad-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 11 00:07:20 2022
tree 4990af9adbfd0fd48501afcea81e1a1aad7e786b
parent 34a090b5730fef3ee8da0570c7d2ce79ec78ba3f

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 <jperaza@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>

client/ios_handler/in_process_intermediate_dump_handler.cc

diff --git a/client/ios_handler/in_process_intermediate_dump_handler.cc b/client/ios_handler/in_process_intermediate_dump_handler.cc
index 88d5eb0..2869c21 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 @@
       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<uint64_t>(image->imageLoadAddress);
     WriteProperty(writer, IntermediateDumpKey::kAddress, &address);
     WriteProperty(
@@ -976,7 +978,12 @@
 
   {
     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<uint64_t>(image_infos->dyldImageLoadAddress);
     WriteProperty(writer, IntermediateDumpKey::kAddress, &address);

snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc

diff --git a/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc b/snapshot/ios/process_snapshot_ios_intermediate_dump_test.cc
index 5f56082..44a7b77 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 @@
     }
   }
 
-  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 @@
     }
   }
 
-  void ExpectModules(const std::vector<const ModuleSnapshot*>& modules) {
+  void ExpectModules(const std::vector<const ModuleSnapshot*>& 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 @@
     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 @@
 
     ExpectSystem(*snapshot.System());
     ExpectThreads(snapshot.Threads());
-    ExpectModules(snapshot.Modules());
+    ExpectModules(snapshot.Modules(), expect_module_path);
     ExpectMachException(*snapshot.Exception());
   }
 
@@ -626,14 +633,14 @@
     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 @@
     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) {