https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/204471
>From a91eba0305efd74e97d99daf37599e2e14abe329 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Tue, 16 Jun 2026 22:30:34 +0100 Subject: [PATCH 1/3] [lldb][Mach-O] Bound the fat header arch loop by available data `ObjectContainerUniversalMachO::ParseHeader` reads the 32-bit `nfat_arch` field straight from the file and uses it as the loop bound when indexing the fat_arch records, without validating it against the amount of data actually present. A crafted fat header that claims nfat_arch = 0xFFFFFFFF while providing only a few bytes makes the loop iterate ~4.29 billion times: `ValidOffsetForDataOfSize()` fails on every iteration past the real data, but the loop keeps spinning up to `nfat_arch`, so the call does not return in any practical amount of time (~20 minutes under ASan). Found by `lldb-target-fuzzer`, reachable from `SBDebugger::CreateTarget` -> `ObjectFile::GetModuleSpecifications` -> `ObjectContainerUniversalMachO::{GetModuleSpecifications,ParseHeader}`. The reduced reproducer is 40 bytes: a `FAT_MAGIC_64` header with `nfat_arch = 0xFFFFFFFF` followed by a single, partial arch record. Break out of the loop as soon as the next fat_arch record no longer fits in the remaining data. This is behavior-preserving, a failed `ValidOffsetForDataOfSize` never advances the offset, so the set of parsed fat_archs is unchanged. It just stops the useless spinning. --- .../Universal-Mach-O/ObjectContainerUniversalMachO.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp index d7b1710dce05d..6193fc5bdd39d 100644 --- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp +++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp @@ -122,6 +122,11 @@ bool ObjectContainerUniversalMachO::ParseHeader( arch.align = extractor.GetU32(&offset); fat_archs.emplace_back(arch); } + } else { + // nfat_arch is read from the file and is untrusted (up to 0xFFFFFFFF). + // Once the data is exhausted the offset can no longer advance, so the + // remaining iterations would spin uselessly; stop here. + break; } } return true; >From 8f6e4598b5062a6664b7c55c7a4ac40553779d26 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Tue, 16 Jun 2026 22:31:46 +0100 Subject: [PATCH 2/3] [lldb][Mach-O] Reject self-referential slices in the fat container `ObjectContainerUniversalMachO::GetModuleSpecifications` re-parses every fat slice by recursing into `ObjectFile::GetModuleSpecifications` at the slice's offset. There is no recursion-depth limit and no check that a slice advances past the current offset, so a slice whose offset is 0 produces a recursive call with identical arguments (same file, same file_offset, same size) and recurses until the stack overflows. Found by `lldb-target-fuzzer`, reachable from `SBDebugger::CreateTarget` -> `ObjectFile::GetModuleSpecifications` -> `ObjectContainerUniversalMachO::GetModuleSpecifications`, which calls back into `ObjectFile::GetModuleSpecifications`. Fixing the `nfat_arch` loop bound (previous commit) unmasked this for the same input: the initial 512-byte read zero-pads a short file, `ParseHeader` then parses a trailing all-zero arch record whose offset is 0, and the slice recurses on itself. Require `slice_file_offset > file_offset` before recursing, so every recursive call strictly advances into the file. Offsets that land at or before the current offset, including 0, and offsets inside the fat header, are skipped. --- .../Universal-Mach-O/ObjectContainerUniversalMachO.cpp | 7 ++++++- 1 file changed, 6 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 6193fc5bdd39d..f3127ef920982 100644 --- a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp +++ b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp @@ -207,7 +207,12 @@ ModuleSpecList ObjectContainerUniversalMachO::GetModuleSpecifications( for (const FatArch &fat_arch : fat_archs) { const lldb::offset_t slice_file_offset = fat_arch.GetOffset() + file_offset; - if (fat_arch.GetOffset() < file_size && file_size > slice_file_offset) { + // The slice must start strictly past the current offset. A slice whose + // offset is 0 (or otherwise does not advance) is self-referential: the + // recursive call below would re-parse the same range and recurse + // without bound. Skip such slices instead of overflowing the stack. + if (slice_file_offset > file_offset && + fat_arch.GetOffset() < file_size && file_size > slice_file_offset) { ModuleSpecList arch_specs = ObjectFile::GetModuleSpecifications( file, slice_file_offset, file_size - slice_file_offset); specs.Append(arch_specs); >From b9547d98ff68663e72ab7beff71903e91e09c830 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Thu, 18 Jun 2026 14:23:12 +0100 Subject: [PATCH 3/3] [lldb][Mach-O] Add unit tests for fat container fuzzer fixes Add `ObjectContainerUniversalMachO` unit tests covering the two `lldb-target-fuzzer` regressions fixed earlier on this branch: - HugeNfatArch: a fat header claiming `nfat_arch = 0xFFFFFFFF` backed by a single real slice must not spin the arch-indexing loop ~4.29 billion times. - SliceOffsetZero: a self-referential slice at offset 0 must be skipped rather than recursing until the stack overflows. Both drive `ObjectFile::GetModuleSpecifications` over an in-memory fat Mach-O built from YAML -- the same "target create" path the fuzzer reached -- and assert it returns promptly with no loadable specs. These live in a new `lldb/unittests/ObjectContainer` directory and replace the API-test versions per review feedback to prefer unit tests. --- lldb/unittests/CMakeLists.txt | 1 + lldb/unittests/ObjectContainer/CMakeLists.txt | 9 ++ .../ObjectContainerUniversalMachOTest.cpp | 119 ++++++++++++++++++ 3 files changed, 129 insertions(+) create mode 100644 lldb/unittests/ObjectContainer/CMakeLists.txt create mode 100644 lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp diff --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt index 41e1c29c8093b..d55bf96f87cba 100644 --- a/lldb/unittests/CMakeLists.txt +++ b/lldb/unittests/CMakeLists.txt @@ -73,6 +73,7 @@ add_subdirectory(Host) add_subdirectory(Instruction) add_subdirectory(Interpreter) add_subdirectory(Language) +add_subdirectory(ObjectContainer) add_subdirectory(ObjectFile) add_subdirectory(Platform) add_subdirectory(Process) diff --git a/lldb/unittests/ObjectContainer/CMakeLists.txt b/lldb/unittests/ObjectContainer/CMakeLists.txt new file mode 100644 index 0000000000000..80778d1c32f17 --- /dev/null +++ b/lldb/unittests/ObjectContainer/CMakeLists.txt @@ -0,0 +1,9 @@ +add_lldb_unittest(ObjectContainerTests + ObjectContainerUniversalMachOTest.cpp + + LINK_LIBS + lldbPluginObjectContainerMachOArchive + lldbCore + lldbUtilityHelpers + LLVMTestingSupport + ) diff --git a/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp new file mode 100644 index 0000000000000..a4346befbfd8b --- /dev/null +++ b/lldb/unittests/ObjectContainer/ObjectContainerUniversalMachOTest.cpp @@ -0,0 +1,119 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.h" +#include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" +#include "lldb/Core/ModuleSpec.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Symbol/ObjectFile.h" +#include "lldb/Utility/FileSpec.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Testing/Support/Error.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +namespace { +class ObjectContainerUniversalMachOTest : public ::testing::Test { + SubsystemRAII<FileSystem, ObjectContainerUniversalMachO> subsystems; +}; +} // namespace + +// Regression fixture: a universal (fat) Mach-O whose header claims +// nfat_arch = 0xFFFFFFFF while the file holds a single arch slice. The arch +// loop in ObjectContainerUniversalMachO::ParseHeader used nfat_arch as its +// bound without checking it against the available data, so this header sent it +// spinning ~4.29 billion times. GetModuleSpecifications must instead stop once +// the data is exhausted and return promptly. Found by lldb-target-fuzzer. +TEST_F(ObjectContainerUniversalMachOTest, HugeNfatArch) { + auto ExpectedFile = TestFile::fromYaml(R"( +--- !fat-mach-o +FatHeader: + magic: 0xCAFEBABF + nfat_arch: 0xFFFFFFFF +FatArchs: + - cputype: 0x01000007 + cpusubtype: 0x00000003 + offset: 0x0000000000004000 + size: 4 + align: 14 + reserved: 0x00000000 +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()); + + ModuleSpec Spec = ExpectedFile->moduleSpec(); + lldb::DataExtractorSP Data = Spec.GetExtractor(); + // Before the fix this loops ~0xFFFFFFFF times and never returns in practice; + // reaching the assertion at all is the regression check. + ModuleSpecList Specs = ObjectFile::GetModuleSpecifications( + Spec.GetFileSpec(), Data, 0, Data->GetByteSize()); + EXPECT_EQ(Specs.GetSize(), 0u); +} + +// Regression fixture: a universal (fat) Mach-O with a self-referential slice +// whose offset is 0. ObjectContainerUniversalMachO::GetModuleSpecifications +// re-parses each slice by recursing into ObjectFile::GetModuleSpecifications at +// the slice offset; a slice at offset 0 produced a recursive call with +// identical arguments and recursed until the stack overflowed. The +// non-advancing slice must be skipped, leaving nothing loadable. Found by +// lldb-target-fuzzer. +TEST_F(ObjectContainerUniversalMachOTest, SliceOffsetZero) { + auto ExpectedFile = TestFile::fromYaml(R"( +--- !fat-mach-o +FatHeader: + magic: 0xCAFEBABE + nfat_arch: 1 +FatArchs: + - cputype: 0x00000007 + cpusubtype: 0x00000003 + offset: 0x00000000 + size: 0x00001000 + align: 12 +Slices: + - !mach-o + FileHeader: + magic: 0xFEEDFACF + cputype: 0x00000007 + 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()); + + // Before the fix the offset-0 slice recurses on identical arguments and + // overflows the stack; reaching the assertion at all is the regression + // check. The self-referential slice is now skipped, so no specs are found. + ModuleSpecList Specs = ObjectFile::GetModuleSpecifications( + FileSpec(TmpFile->TmpName), /*file_offset=*/0, /*file_size=*/0); + EXPECT_EQ(Specs.GetSize(), 0u); + + ASSERT_THAT_ERROR(TmpFile->discard(), llvm::Succeeded()); +} _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
