[Lldb-commits] [lldb] [lldb] Bounds check minidump location descriptors (PR #223911)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/223911

MinidumpParser::GetModuleUUID sliced the file with the RVA and DataSize taken straight from a module's CvRecord. ArrayRef::slice asserts, but a build without assertions still reads out of bounds.

Route both through MinidumpFile::getRawData, which range checks in 64 bits, and log the error rather than returning a slice that looks valid.

rdar://186889966

>From e8ec435c32babb3f85390d1270ade9e34ef44606 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 15 Sep 2026 22:14:45 -0700
Subject: [PATCH] [lldb] Bounds check minidump location descriptors

MinidumpParser::GetModuleUUID sliced the file with the RVA and DataSize
taken straight from a module's CvRecord. ArrayRef::slice asserts, but a
build without assertions still reads out of bounds.

Route both through MinidumpFile::getRawData, which range checks in 64
bits, and log the error rather than returning a slice that looks valid.

rdar://186889966
---
 .../Process/minidump/MinidumpParser.cpp       | 20 ++++--
 .../Process/minidump/MinidumpParserTest.cpp   | 68 +++++++++++++++++++
 2 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index 1334f69dad6dc..f15e837cc006c 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -55,8 +55,14 @@ MinidumpParser::GetRawStream(StreamType stream_type) {
 }
 
 UUID MinidumpParser::GetModuleUUID(const minidump::Module *module) {
-  auto cv_record =
-      GetData().slice(module->CvRecord.RVA, module->CvRecord.DataSize);
+  llvm::Expected<llvm::ArrayRef<uint8_t>> expected_cv_record =
+      GetMinidumpFile().getRawData(module->CvRecord);
+  if (!expected_cv_record) {
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Modules), expected_cv_record.takeError(),
+                   "Failed to read the CodeView record: {0}");
+    return UUID();
+  }
+  llvm::ArrayRef<uint8_t> cv_record = *expected_cv_record;
 
   // Read the CV record signature
   const llvm::support::ulittle32_t *signature = nullptr;
@@ -96,9 +102,15 @@ llvm::ArrayRef<minidump::Thread> MinidumpParser::GetThreads() {
 
 llvm::ArrayRef<uint8_t>
 MinidumpParser::GetThreadContext(const LocationDescriptor &location) {
-  if (location.RVA + location.DataSize > GetData().size())
+  // Use getRawData to widen the two 32-bit fields and check for overflow.
+  llvm::Expected<llvm::ArrayRef<uint8_t>> expected_context =
+      GetMinidumpFile().getRawData(location);
+  if (!expected_context) {
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), expected_context.takeError(),
+                   "Failed to read the thread context: {0}");
     return {};
-  return GetData().slice(location.RVA, location.DataSize);
+  }
+  return *expected_context;
 }
 
 llvm::ArrayRef<uint8_t>
diff --git a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
index a5986eafda83e..2121e3b7f9f97 100644
--- a/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
+++ b/lldb/unittests/Process/minidump/MinidumpParserTest.cpp
@@ -36,6 +36,28 @@
 using namespace lldb_private;
 using namespace minidump;
 
+/// Wrap raw stream contents in a single-stream minidump. Unlike yaml2obj, this
+/// leaves the location descriptors inside the stream untouched, so they can
+/// point anywhere.
+static std::string MakeMinidump(StreamType type, llvm::StringRef contents) {
+  Header header = {};
+  header.Signature = Header::MagicSignature;
+  header.Version = Header::MagicVersion;
+  header.NumberOfStreams = 1;
+  header.StreamDirectoryRVA = sizeof(Header);
+
+  Directory directory = {};
+  directory.Type = type;
+  directory.Location.RVA = sizeof(Header) + sizeof(Directory);
+  directory.Location.DataSize = contents.size();
+
+  std::string data;
+  data.append(reinterpret_cast<const char *>(&header), sizeof(header));
+  data.append(reinterpret_cast<const char *>(&directory), sizeof(directory));
+  data.append(contents);
+  return data;
+}
+
 class MinidumpParserTest : public testing::Test {
 public:
   SubsystemRAII<FileSystem> subsystems;
@@ -59,6 +81,10 @@ class MinidumpParserTest : public testing::Test {
       return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                      "convertYAML() failed");
 
+    return SetUpFromData(data);
+  }
+
+  llvm::Error SetUpFromData(llvm::StringRef data) {
     auto data_buffer_sp =
         std::make_shared<DataBufferHeap>(data.data(), data.size());
     auto expected_parser = MinidumpParser::Create(std::move(data_buffer_sp));
@@ -911,3 +937,45 @@ TEST_F(MinidumpParserTest, MinidumpModuleOrder) {
       parser->GetMinidumpFile().getString(filtered_modules[1]->ModuleNameRVA),
       llvm::HasValue("/tmp/b"));
 }
+
+TEST_F(MinidumpParserTest, GetModuleUUIDOutOfRangeCvRecord) {
+  minidump::Module module = {};
+  module.BaseOfImage = 0x1000;
+  module.SizeOfImage = 0x1000;
+  module.CvRecord.RVA = 0xf0000000;
+  module.CvRecord.DataSize = sizeof(llvm::support::ulittle32_t);
+
+  std::string stream;
+  llvm::support::ulittle32_t module_count(1);
+  stream.append(reinterpret_cast<const char *>(&module_count),
+                sizeof(module_count));
+  stream.append(reinterpret_cast<const char *>(&module), sizeof(module));
+
+  ASSERT_THAT_ERROR(SetUpFromData(MakeMinidump(StreamType::ModuleList, stream)),
+                    llvm::Succeeded());
+
+  llvm::ArrayRef<minidump::Module> modules = parser->GetModuleList();
+  ASSERT_EQ(1u, modules.size());
+  EXPECT_FALSE(parser->GetModuleUUID(&modules[0]).IsValid());
+}
+
+TEST_F(MinidumpParserTest, GetThreadContextOutOfRange) {
+  minidump::Thread thread = {};
+  thread.ThreadId = 0x3e81;
+  // The sum of the two fields wraps around in 32 bits.
+  thread.Context.RVA = 0xfffffff0;
+  thread.Context.DataSize = 0x20;
+
+  std::string stream;
+  llvm::support::ulittle32_t thread_count(1);
+  stream.append(reinterpret_cast<const char *>(&thread_count),
+                sizeof(thread_count));
+  stream.append(reinterpret_cast<const char *>(&thread), sizeof(thread));
+
+  ASSERT_THAT_ERROR(SetUpFromData(MakeMinidump(StreamType::ThreadList, stream)),
+                    llvm::Succeeded());
+
+  llvm::ArrayRef<minidump::Thread> threads = parser->GetThreads();
+  ASSERT_EQ(1u, threads.size());
+  EXPECT_TRUE(parser->GetThreadContext(threads[0]).empty());
+}