[llvm] [DebugInfo][DWARF] Bounds-check the unit-index parallel table (PR #213285)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-debuginfo

Author: Matt Davis (enferex)

<details>
<summary>Changes</summary>

This is one of a number of debug info related defects identified by an LLM based security scan.  It's a relatively trivial bounds-check on the Index number stored in the .debug_cu_index Index table.  It seems reasonable to be more defensive when reading in binary data, especially so when the fix is trivial.  The repro is rather contrived but proves the issue exists. 

---
Full diff: https://github.com/llvm/llvm-project/pull/213285.diff


3 Files Affected:

- (modified) llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp (+2) 
- (modified) llvm/unittests/DebugInfo/DWARF/CMakeLists.txt (+1) 
- (added) llvm/unittests/DebugInfo/DWARF/DWARFUnitIndexTest.cpp (+63) 


``````````diff
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp b/llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
index 08abcf13d0618..f5d7b1e19dd11 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
@@ -159,6 +159,8 @@ bool DWARFUnitIndex::parseImpl(DataExtractor IndexData) {
     auto Index = IndexData.getU32(&Offset);
     if (!Index)
       continue;
+    if (Index > Header.NumUnits)
+      return false;
     Rows[i].Index = this;
     Rows[i].Contributions =
         std::make_unique<Entry::SectionContribution[]>(Header.NumColumns);
diff --git a/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt b/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
index 167f1b310cbc6..98c4e750176cc 100644
--- a/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
+++ b/llvm/unittests/DebugInfo/DWARF/CMakeLists.txt
@@ -28,6 +28,7 @@ add_llvm_unittest(DebugInfoDWARFTests
   DWARFExpressionCompactPrinterTest.cpp
   DWARFFormValueTest.cpp
   DWARFListTableTest.cpp
+  DWARFUnitIndexTest.cpp
   DWARFLocationExpressionTest.cpp
   DWARFVerifierTest.cpp
   )
diff --git a/llvm/unittests/DebugInfo/DWARF/DWARFUnitIndexTest.cpp b/llvm/unittests/DebugInfo/DWARF/DWARFUnitIndexTest.cpp
new file mode 100644
index 0000000000000..b11f508dd8328
--- /dev/null
+++ b/llvm/unittests/DebugInfo/DWARF/DWARFUnitIndexTest.cpp
@@ -0,0 +1,63 @@
+//===- llvm/unittest/DebugInfo/DWARF/DWARFUnitIndexTest.cpp ---------------===//
+//
+// 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 "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/Endian.h"
+#include "gtest/gtest.h"
+#include <cstdint>
+#include <vector>
+
+using namespace llvm;
+
+namespace {
+
+static void appendU32(std::vector<uint8_t> &B, uint32_t V) {
+  uint8_t Tmp[4];
+  support::endian::write32le(Tmp, V);
+  B.insert(B.end(), Tmp, Tmp + 4);
+}
+
+static void appendU64(std::vector<uint8_t> &B, uint64_t V) {
+  uint8_t Tmp[8];
+  support::endian::write64le(Tmp, V);
+  B.insert(B.end(), Tmp, Tmp + 8);
+}
+
+// A crafted .debug_cu_index (version 2) with one hash bucket whose parallel-
+// table Index is NumUnits + 1. Without the bounds check parseImpl() writes
+// Contribs[Index - 1], one slot past a heap array sized to NumUnits. parse()
+// must reject it and leave the index empty.
+TEST(DWARFUnitIndexTest, RejectsRowIndexBeyondNumUnits) {
+  std::vector<uint8_t> Buffer;
+  // Header: Version=2, NumColumns=1, NumUnits=1, NumBuckets=1.
+  appendU32(Buffer, 2);
+  appendU32(Buffer, 1);
+  appendU32(Buffer, 1);
+  appendU32(Buffer, 1);
+  // Hash table of signatures: NumBuckets x u64.
+  appendU64(Buffer, 0xdeadbeefULL);
+  // Parallel table of indexes: NumBuckets x u32. Index = NumUnits + 1 = 2.
+  appendU32(Buffer, 2);
+  // Column headers: NumColumns x u32 (DW_SECT_INFO serializes to 1 under v2).
+  appendU32(Buffer, 1);
+  // Table of section offsets: NumUnits*NumColumns x u32.
+  appendU32(Buffer, 0);
+  // Table of section sizes: NumUnits*NumColumns x u32.
+  appendU32(Buffer, 0);
+
+  DataExtractor Data(
+      StringRef(reinterpret_cast<const char *>(Buffer.data()), Buffer.size()),
+      /*IsLittleEndian=*/true);
+  DWARFUnitIndex Index(DW_SECT_INFO);
+  EXPECT_FALSE(Index.parse(Data));
+  // On failure parse() resets the header, so the index is falsy.
+  EXPECT_FALSE(static_cast<bool>(Index));
+}
+
+} // end anonymous namespace

``````````

</details>


https://github.com/llvm/llvm-project/pull/213285