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