llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Yao Qi (qiyao)
<details>
<summary>Changes</summary>
This fixes two denial-of-service issues in the universal (fat) Mach-O
container parser, both found by `lldb-target-fuzzer` and both reachable from
`SBDebugger::CreateTarget` → `ObjectFile::GetModuleSpecifications` →
`ObjectContainerUniversalMachO::{GetModuleSpecifications,ParseHeader}`.
### 1. Bound the fat header arch loop by available data
`ParseHeader` read the untrusted 32-bit `nfat_arch` field and used it directly
as the loop bound when indexing the `fat_arch` records, without validating it
against the data actually present. A crafted header claiming
`nfat_arch = 0xFFFFFFFF` with only a few bytes of payload made the loop spin
~4.29 billion times — `ValidOffsetForDataOfSize()` fails on every iteration
past the real data, but the loop keeps going up to `nfat_arch`, so the call
never returns in practice (~20 minutes under ASan). The reduced reproducer is
40 bytes.
Fix: 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 advanced the offset, so the set of parsed
`fat_archs` is unchanged; it just stops the useless spinning.
### 2. Reject self-referential slices in the fat container
`GetModuleSpecifications` re-parses every fat slice by recursing into
`ObjectFile::GetModuleSpecifications` at the slice's offset, with no
recursion-depth limit and no check that a slice advances past the current
offset. A slice whose offset is 0 produces a recursive call with identical
arguments (same file, same offset, same size) and recurses until the stack
overflows. Fixing the `nfat_arch` loop bound above 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.
Fix: require `slice_file_offset > file_offset` before recursing, so every
recursive call strictly advances into the file. Offsets at or before the
current offset (including 0) and offsets inside the fat header are skipped.
### Tests
Two API tests in `TestTargetCommand.py`, each with a `yaml2obj` fixture:
- `fat_nfat_arch_huge.yaml` — `nfat_arch = 0xFFFFFFFF` with a single real
slice; `target create` must complete promptly.
- `fat_slice_offset_zero.yaml` — a self-referential slice at offset 0;
`target create` must skip it and fail gracefully ("is not a valid
executable") rather than crashing.
---
Full diff: https://github.com/llvm/llvm-project/pull/204471.diff
4 Files Affected:
- (modified)
lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
(+11-1)
- (modified) lldb/test/API/commands/target/basic/TestTargetCommand.py (+26)
- (added) lldb/test/API/commands/target/basic/fat_nfat_arch_huge.yaml (+30)
- (added) lldb/test/API/commands/target/basic/fat_slice_offset_zero.yaml (+30)
``````````diff
diff --git
a/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
b/lldb/source/Plugins/ObjectContainer/Universal-Mach-O/ObjectContainerUniversalMachO.cpp
index d7b1710dce05d..f3127ef920982 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;
@@ -202,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);
diff --git a/lldb/test/API/commands/target/basic/TestTargetCommand.py
b/lldb/test/API/commands/target/basic/TestTargetCommand.py
index dd6bdb234108a..f6870dc2e7efe 100644
--- a/lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ b/lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -147,6 +147,32 @@ def test_target_create_unsupported_platform(self):
patterns=["error: no matching platforms found for this file"],
)
+ @no_debug_info_test
+ def test_target_create_fat_macho_huge_nfat_arch(self):
+ """A fat Mach-O header whose nfat_arch is larger than the data must not
+ send the arch-indexing loop spinning."""
+ yaml = os.path.join(self.getSourceDir(), "fat_nfat_arch_huge.yaml")
+ exe = self.getBuildArtifact("fat_nfat_arch_huge")
+ self.yaml2obj(yaml, exe)
+ # Before the fix this loops ~0xFFFFFFFF times and test timeout. The
+ # test "passing" here just means target create completes promptly.
+ self.expect("target create {}".format(exe))
+
+ @no_debug_info_test
+ def test_target_create_fat_macho_self_referential_slice(self):
+ """A fat Mach-O slice at offset 0 is self-referential. Parsing it must
+ not recurse until the stack overflows."""
+ yaml = os.path.join(self.getSourceDir(), "fat_slice_offset_zero.yaml")
+ exe = self.getBuildArtifact("fat_slice_offset_zero")
+ self.yaml2obj(yaml, exe)
+ # Before the fix this recurses on identical arguments and crashes. The
+ # non-advancing slice is now skipped, leaving nothing loadable.
+ self.expect(
+ "target create {}".format(exe),
+ error=True,
+ patterns=["is not a valid executable"],
+ )
+
@no_debug_info_test
def test_target_create_invalid_platform(self):
self.buildB()
diff --git a/lldb/test/API/commands/target/basic/fat_nfat_arch_huge.yaml
b/lldb/test/API/commands/target/basic/fat_nfat_arch_huge.yaml
new file mode 100644
index 0000000000000..4052d044d2449
--- /dev/null
+++ b/lldb/test/API/commands/target/basic/fat_nfat_arch_huge.yaml
@@ -0,0 +1,30 @@
+--- !fat-mach-o
+# 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. "target create" must instead parse the one
+# real slice and return promptly. Found by lldb-target-fuzzer.
+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: []
+...
diff --git a/lldb/test/API/commands/target/basic/fat_slice_offset_zero.yaml
b/lldb/test/API/commands/target/basic/fat_slice_offset_zero.yaml
new file mode 100644
index 0000000000000..ea5d120026791
--- /dev/null
+++ b/lldb/test/API/commands/target/basic/fat_slice_offset_zero.yaml
@@ -0,0 +1,30 @@
+--- !fat-mach-o
+# Regression fixture: a universal (fat) Mach-O with a self-referential slice
+# whose offset is 0. ObjectContainerUniversalMachO::GetModuleSpecifications
+# re-parsed 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. "target create"
+# must skip the non-advancing slice and fail gracefully instead of crashing.
+# Found by lldb-target-fuzzer.
+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: []
+...
``````````
</details>
https://github.com/llvm/llvm-project/pull/204471
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
