https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/221493
>From 0aa0600632f5c32f9a6e3a251b2c8806d3308493 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Tue, 1 Sep 2026 09:40:14 +0100 Subject: [PATCH 1/2] [lldb] Validate universal Mach-O slice bounds before parsing Opening a corrupt universal (fat) Mach-O file can crash lldb. A fat-arch header entry's (offset, size) pair is untrusted, and `GetObjectFile()` passes it straight to `ObjectFile::FindPlugin()` with no check against the container's own size. A slice whose offset is 0 points back at the start of the same fat header, so `GetObjectFile()` recurses into itself until the stack is exhausted: ``` $ ./ObjectContainerTests --gtest_filter=GetObjectFileSelfReferentialSlice #230 ObjectContainerUniversalMachO::GetObjectFile(FileSpec const*) ObjectContainerUniversalMachO.cpp:188 #231 CreateObjectFromContainer(...) ObjectFile.cpp:43 #232 lldb_private::ObjectFile::FindPlugin(...) ObjectFile.cpp:146 #233 ObjectContainerUniversalMachO::GetObjectFile(FileSpec const*) ObjectContainerUniversalMachO.cpp:188 (repeats until the stack is exhausted) SUMMARY: AddressSanitizer: stack-overflow (libsystem_trace.dylib:arm64e+0x16bd8) in os_log_fmt_flatten+0x6d4 ``` A slice whose declared size is far larger than the file reaches the allocator instead, with the untrusted size unchanged: ``` $ ./ObjectContainerTests --gtest_filter=GetObjectFileOversizedSlice ==89958==ERROR: AddressSanitizer: requested allocation size 0xffffffffffff0046 (0xffffffffffff1048 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0) #9 ObjectContainerUniversalMachO::GetObjectFile(FileSpec const*) ObjectContainerUniversalMachO.cpp:188 SUMMARY: AddressSanitizer: allocation-size-too-big MemoryBuffer.cpp:343 in llvm::WritableMemoryBuffer::getNewUninitMemBuffer(unsigned long, llvm::Twine const&, std::__1::optional<llvm::Align>) ``` `GetModuleSpecifications()` in the same file already guards its own recursion this way: it only follows a slice that starts strictly past the container's current offset, and clamps the recursive call to the bytes remaining in the container. Apply the same check to `GetObjectFile()`. Found with lldb-target-fuzzer. The commands above are the two regression tests added here. --- .../ObjectContainerUniversalMachO.cpp | 20 +++-- lldb/unittests/ObjectContainer/CMakeLists.txt | 1 + .../ObjectContainerUniversalMachOTest.cpp | 90 ++++++++++++++++++- 3 files changed, 105 insertions(+), 6 deletions(-) diff --git a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp index 1fcf7dd882bf4..a929bdc8a4640 100644 --- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp +++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp @@ -16,6 +16,8 @@ #include "lldb/Utility/DataBuffer.h" #include "lldb/Utility/Stream.h" +#include <algorithm> + using namespace lldb; using namespace lldb_private; using namespace llvm::MachO; @@ -183,11 +185,19 @@ ObjectContainerUniversalMachO::GetObjectFile(const FileSpec *file) { } if (arch_idx < m_header.nfat_arch) { - DataExtractorSP extractor_sp; - lldb::offset_t data_offset = 0; - return ObjectFile::FindPlugin( - module_sp, file, m_offset + m_fat_archs[arch_idx].GetOffset(), - m_fat_archs[arch_idx].GetSize(), extractor_sp, data_offset); + const FatArch &fat_arch = m_fat_archs[arch_idx]; + const uint64_t byte_size = GetByteSize(); + // The slice must start strictly past this container's own bytes and + // fit within them. + if (fat_arch.GetOffset() > 0 && fat_arch.GetOffset() < byte_size) { + DataExtractorSP extractor_sp; + lldb::offset_t data_offset = 0; + const uint64_t slice_size = + std::min(fat_arch.GetSize(), byte_size - fat_arch.GetOffset()); + return ObjectFile::FindPlugin(module_sp, file, + m_offset + fat_arch.GetOffset(), + slice_size, extractor_sp, data_offset); + } } } return ObjectFileSP(); diff --git a/lldb/unittests/ObjectContainer/CMakeLists.txt b/lldb/unittests/ObjectContainer/CMakeLists.txt index 8acfccf12f4dd..e4ca2df0202a9 100644 --- a/lldb/unittests/ObjectContainer/CMakeLists.txt +++ b/lldb/unittests/ObjectContainer/CMakeLists.txt @@ -4,6 +4,7 @@ add_lldb_unittest(ObjectContainerTests LINK_LIBS lldbPluginObjectContainerMachOArchive lldbPluginObjectContainerMachOFileset + lldbPluginObjectFileMachO lldbCore lldbUtilityHelpers LLVMTestingSupport diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp index 2d4cbacc5169f..e1a372d954062 100644 --- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp +++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp @@ -8,8 +8,10 @@ #include "Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h" #include "Plugins/ObjectContainer/Mach-O-Fileset/ObjectContainerMachOFileset.h" +#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h" #include "TestingSupport/SubsystemRAII.h" #include "TestingSupport/TestUtilities.h" +#include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Host/FileSystem.h" #include "lldb/Symbol/ObjectFile.h" @@ -17,15 +19,20 @@ #include "lldb/Utility/DataBufferHeap.h" #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/FileSpec.h" +#include "llvm/BinaryFormat/MachO.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/raw_ostream.h" #include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" +#include <vector> + using namespace lldb_private; namespace { class ObjectContainerUniversalMachOTest : public ::testing::Test { - SubsystemRAII<FileSystem, ObjectContainerUniversalMachO> subsystems; + SubsystemRAII<FileSystem, ObjectContainerUniversalMachO, ObjectFileMachO> + subsystems; }; } // namespace @@ -122,6 +129,87 @@ TEST_F(ObjectContainerUniversalMachOTest, SliceOffsetZero) { ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded()); } +// A fat Mach-O slice at offset 0 is self-referential. +TEST_F(ObjectContainerUniversalMachOTest, GetObjectFileSelfReferentialSlice) { + auto ExpectedFile = TestFile::fromYaml(R"( +--- !fat-mach-o +FatHeader: + magic: 0xCAFEBABE + nfat_arch: 1 +FatArchs: + - cputype: 0x01000007 + cpusubtype: 0x00000003 + offset: 0x00000000 + size: 0x00001000 + align: 12 +Slices: + - !mach-o + FileHeader: + magic: 0xFEEDFACF + cputype: 0x01000007 + cpusubtype: 0x00000003 + filetype: 0x00000002 + ncmds: 0 + sizeofcmds: 0 + flags: 0x00000000 + reserved: 0x00000000 + LoadCommands: [] +... +)"); + ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded()); + + llvm::Expected<llvm::sys::fs::TempFile> TmpFile = + ExpectedFile->writeToTemporaryFile(); + ASSERT_THAT_EXPECTED(TmpFile, llvm::Succeeded()); + + ArchSpec Arch; + Arch.SetArchitecture(eArchTypeMachO, 0x01000007, 0x00000003); + lldb::ModuleSP Module = + std::make_shared<lldb_private::Module>(FileSpec(TmpFile->TmpName), Arch); + EXPECT_EQ(Module->GetObjectFile(), nullptr); + + ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded()); +} + +// A fat Mach-O slice whose declared size is far larger than the file. +TEST_F(ObjectContainerUniversalMachOTest, GetObjectFileOversizedSlice) { + std::vector<uint8_t> Data = { + 0xCA, 0xFE, 0xBA, 0xBF, // magic: FAT_MAGIC_64 + 0x00, 0x00, 0x00, 0x01, // nfat_arch: 1 + 0x01, 0x00, 0x00, 0x07, // cputype: X86_64 + 0x00, 0x00, 0x00, 0x03, // cpusubtype: 3 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28, // offset: 40 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, // size: 2^48 - 1 + 0x00, 0x00, 0x00, 0x0C, // align: 12 + 0x00, 0x00, 0x00, 0x00, // reserved: 0 + }; + // It is the plugin claiming this slice that goes on to request the declared + // size above. + llvm::MachO::mach_header_64 SliceHeader = {}; + SliceHeader.magic = llvm::MachO::MH_MAGIC_64; + SliceHeader.cputype = llvm::MachO::CPU_TYPE_X86_64; + SliceHeader.cpusubtype = 3; + SliceHeader.filetype = llvm::MachO::MH_EXECUTE; + const auto *SliceBytes = reinterpret_cast<const uint8_t *>(&SliceHeader); + Data.insert(Data.end(), SliceBytes, SliceBytes + sizeof(SliceHeader)); + Data.resize(40 + 512, 0); + + llvm::Expected<llvm::sys::fs::TempFile> TmpFile = + llvm::sys::fs::TempFile::create("temp%%%%%%%%%%%%%%%%"); + ASSERT_THAT_EXPECTED(TmpFile, llvm::Succeeded()); + llvm::raw_fd_ostream(TmpFile->FD, /*shouldClose=*/false) << llvm::StringRef( + reinterpret_cast<const char *>(Data.data()), Data.size()); + + ArchSpec Arch; + Arch.SetArchitecture(eArchTypeMachO, 0x01000007, 0x00000003); + lldb::ModuleSP Module = + std::make_shared<lldb_private::Module>(FileSpec(TmpFile->TmpName), Arch); + ObjectFile *Obj = Module->GetObjectFile(); + ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded()); + ASSERT_NE(Obj, nullptr); + EXPECT_EQ(Obj->GetByteSize(), 512u); // Clamped to the bytes available. +} + // Regression fixture: a Mach-O fileset whose single load command has // cmdsize = 0. With ncmds set near INT_MAX the function hangs. The // fix breaks out of the loop as soon as >From b84b67fe11f9e565902668bc44bc1222eaf48ec7 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Sat, 5 Sep 2026 23:44:13 +0100 Subject: [PATCH 2/2] [lldb] Fix GetObjectFileOversizedSlice on Windows The test failed on Windows: ``` ObjectContainerUniversalMachOTest.cpp(209): error: Expected: (Obj) != (nullptr), actual: NULL vs (nullptr) ``` `llvm::sys::fs::TempFile::create` marks the file delete-pending on Windows, and a delete-pending file cannot be reopened by name. The test needs the opposite: `GetObjectFile()` re-reads the slice from the path, so the file has to stay openable. Use `createTemporaryFile` with an explicit `remove` instead. --- .../ObjectContainerUniversalMachOTest.cpp | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp index e1a372d954062..bbb0c347ad7ea 100644 --- a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp +++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp @@ -194,20 +194,28 @@ TEST_F(ObjectContainerUniversalMachOTest, GetObjectFileOversizedSlice) { Data.insert(Data.end(), SliceBytes, SliceBytes + sizeof(SliceHeader)); Data.resize(40 + 512, 0); - llvm::Expected<llvm::sys::fs::TempFile> TmpFile = - llvm::sys::fs::TempFile::create("temp%%%%%%%%%%%%%%%%"); - ASSERT_THAT_EXPECTED(TmpFile, llvm::Succeeded()); - llvm::raw_fd_ostream(TmpFile->FD, /*shouldClose=*/false) << llvm::StringRef( - reinterpret_cast<const char *>(Data.data()), Data.size()); + // The slice is re-read from the file path, so the bytes must be on disk and + // the path must stay openable. + int FD; + llvm::SmallString<128> TmpName; + ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("oversized-slice", "bin", FD, + TmpName)); + { + llvm::raw_fd_ostream OS(FD, /*shouldClose=*/true); + OS << llvm::StringRef(reinterpret_cast<const char *>(Data.data()), + Data.size()); + } ArchSpec Arch; Arch.SetArchitecture(eArchTypeMachO, 0x01000007, 0x00000003); lldb::ModuleSP Module = - std::make_shared<lldb_private::Module>(FileSpec(TmpFile->TmpName), Arch); + std::make_shared<lldb_private::Module>(FileSpec(TmpName), Arch); ObjectFile *Obj = Module->GetObjectFile(); - ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded()); ASSERT_NE(Obj, nullptr); EXPECT_EQ(Obj->GetByteSize(), 512u); // Clamped to the bytes available. + + Module.reset(); + ASSERT_FALSE(llvm::sys::fs::remove(TmpName)); } // Regression fixture: a Mach-O fileset whose single load command has _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
