[Lldb-commits] [lldb] [lldb][Mach-O] Bounds-check GetArchitectureAtIndex against m_fat_archs (PR #205289)

[Yao Qi via lldb-commits]

https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/205289

>From c36aa4aac976f80f26e69e1b75d9528e2f139c7c Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Mon, 22 Jun 2026 21:52:21 +0100
Subject: [PATCH 1/3] [lldb][Mach-O] Bounds-check GetArchitectureAtIndex
 against m_fat_archs

`ObjectContainerUniversalMachO::GetArchitectureAtIndex` used
`m_header.nfat_arch` (read directly from the file and untrusted, up to
0xFFFFFFFF) as the bound before indexing `m_fat_archs`.  When ParseHeader
exhausts the data partway through and breaks early, `m_fat_archs.size()`
can be smaller than `nfat_arch`, so the indexed load is out of bounds.
Bound the check on the actual vector size instead.

Found by lldb-target-fuzzer.
---
 .../ObjectContainerUniversalMachO.cpp         |  4 ++-
 .../ObjectContainerUniversalMachOTest.cpp     | 28 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
index f3127ef920982..363cc47e59662 100644
--- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
+++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
@@ -142,7 +142,9 @@ size_t ObjectContainerUniversalMachO::GetNumArchitectures() const {
 
 bool ObjectContainerUniversalMachO::GetArchitectureAtIndex(
     uint32_t idx, ArchSpec &arch) const {
-  if (idx < m_header.nfat_arch) {
+  // guard against m_fat_archs.size() to keep this safe regardless of
+  // how the header was populated.
+  if (idx < m_fat_archs.size()) {
     arch.SetArchitecture(eArchTypeMachO, m_fat_archs[idx].GetCPUType(),
                          m_fat_archs[idx].GetCPUSubType());
     return true;
diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
index a4346befbfd8b..25262952f20ef 100644
--- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
+++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
@@ -12,6 +12,8 @@
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/FileSpec.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Testing/Support/Error.h"
@@ -117,3 +119,29 @@ TEST_F(ObjectContainerUniversalMachOTest, SliceOffsetZero) {
 
   ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded());
 }
+
+// Regression fixture: a universal (fat) Mach-O whose header claims a huge
+// nfat_arch (here 0xAFAFAFAF) but provides no fat_arch entries beyond the
+// header bytes.  Found by lldb-target-fuzzer.
+TEST_F(ObjectContainerUniversalMachOTest, NfatArchTruncatedSlices) {
+  // Hand-crafted fat header: FAT_MAGIC_64 + nfat_arch=0xAFAFAFAF + 2 stray
+  // payload bytes, not enough for even one fat_arch_64 entry (32 bytes).
+  const uint8_t kData[] = {
+      0xCA, 0xFE, 0xBA, 0xBF, // magic:     FAT_MAGIC_64 (big endian)
+      0xAF, 0xAF, 0xAF, 0xAF, // nfat_arch: 0xAFAFAFAF (untrusted, huge)
+      0xAF, 0xAF,             // truncated arch payload
+  };
+  lldb::DataBufferSP Buf =
+      std::make_shared<DataBufferHeap>(kData, sizeof(kData));
+
+  std::unique_ptr<lldb_private::ObjectContainer> Container(
+      ObjectContainerUniversalMachO::CreateInstance(
+          /*module_sp=*/nullptr, Buf, /*data_offset=*/0, /*file=*/nullptr,
+          /*file_offset=*/0, /*length=*/sizeof(kData)));
+  ASSERT_NE(Container.get(), nullptr);
+
+  // Before the fix, this m_fat_archs[0] causes an OOB std::vector access; after
+  // the fix the bounds check against m_fat_archs.size() returns false.
+  ArchSpec Arch;
+  EXPECT_FALSE(Container->GetArchitectureAtIndex(0, Arch));
+}

>From aec662508b5ef0cfe0b6eab3bf603571ec509e1f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Wed, 24 Jun 2026 08:44:36 +0100
Subject: [PATCH 2/3] Remove unnecessary comments

---
 .../Universal-Mach-O/ObjectContainerUniversalMachO.cpp         | 2 --
 .../ObjectContainer/ObjectContainerUniversalMachOTest.cpp      | 3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
index 363cc47e59662..1fcf7dd882bf4 100644
--- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
+++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
@@ -142,8 +142,6 @@ size_t ObjectContainerUniversalMachO::GetNumArchitectures() const {
 
 bool ObjectContainerUniversalMachO::GetArchitectureAtIndex(
     uint32_t idx, ArchSpec &arch) const {
-  // guard against m_fat_archs.size() to keep this safe regardless of
-  // how the header was populated.
   if (idx < m_fat_archs.size()) {
     arch.SetArchitecture(eArchTypeMachO, m_fat_archs[idx].GetCPUType(),
                          m_fat_archs[idx].GetCPUSubType());
diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
index 25262952f20ef..a5080886a9e29 100644
--- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
+++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
@@ -140,8 +140,7 @@ TEST_F(ObjectContainerUniversalMachOTest, NfatArchTruncatedSlices) {
           /*file_offset=*/0, /*length=*/sizeof(kData)));
   ASSERT_NE(Container.get(), nullptr);
 
-  // Before the fix, this m_fat_archs[0] causes an OOB std::vector access; after
-  // the fix the bounds check against m_fat_archs.size() returns false.
+  // m_fat_archs has zero emlement, returns false.
   ArchSpec Arch;
   EXPECT_FALSE(Container->GetArchitectureAtIndex(0, Arch));
 }

>From 6944d64534f4b9cab3b7e19630bd24b2cb38a2a7 Mon Sep 17 00:00:00 2001
From: Yao Qi <qiyaoltc at gmail.com>
Date: Wed, 24 Jun 2026 21:48:18 +0100
Subject: [PATCH 3/3] Update
 lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp

Co-authored-by: Alex Langford <nirvashtzero at gmail.com>
---
 .../ObjectContainer/ObjectContainerUniversalMachOTest.cpp       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
index a5080886a9e29..50ab4c9557b66 100644
--- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
+++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp
@@ -140,7 +140,7 @@ TEST_F(ObjectContainerUniversalMachOTest, NfatArchTruncatedSlices) {
           /*file_offset=*/0, /*length=*/sizeof(kData)));
   ASSERT_NE(Container.get(), nullptr);
 
-  // m_fat_archs has zero emlement, returns false.
+  // m_fat_archs has zero elements, returns false.
   ArchSpec Arch;
   EXPECT_FALSE(Container->GetArchitectureAtIndex(0, Arch));
 }