llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Yao Qi (qiyao) <details> <summary>Changes</summary> `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. --- Full diff: https://github.com/llvm/llvm-project/pull/205289.diff 2 Files Affected: - (modified) lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp (+3-1) - (modified) lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp (+28) ``````````diff 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)); +} `````````` </details> https://github.com/llvm/llvm-project/pull/205289 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
