[llvm][ELF]Add Shdr check for getBuildID

llvm/include/llvm/Object/ELF.h

@@ -407,7 +407,8 @@ class ELFFile {
   Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const {
     assert(Phdr.p_type == ELF::PT_NOTE && "Phdr is not of type PT_NOTE");
     ErrorAsOutParameter ErrAsOutParam(Err);
-    if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) {
+    if (Phdr.p_offset + Phdr.p_filesz > getBufSize() ||
+        Phdr.p_offset + Phdr.p_filesz < Phdr.p_offset) {
       Err =
           createError("invalid offset (0x" + Twine::utohexstr(Phdr.p_offset) +
                       ") or size (0x" + Twine::utohexstr(Phdr.p_filesz) + ")");
@@ -435,7 +436,8 @@ class ELFFile {
   Elf_Note_Iterator notes_begin(const Elf_Shdr &Shdr, Error &Err) const {
     assert(Shdr.sh_type == ELF::SHT_NOTE && "Shdr is not of type SHT_NOTE");
     ErrorAsOutParameter ErrAsOutParam(Err);
-    if (Shdr.sh_offset + Shdr.sh_size > getBufSize()) {
+    if (Shdr.sh_offset + Shdr.sh_size > getBufSize() ||
+        Shdr.sh_offset + Shdr.sh_size < Shdr.sh_offset) {
       Err =
           createError("invalid offset (0x" + Twine::utohexstr(Shdr.sh_offset) +
                       ") or size (0x" + Twine::utohexstr(Shdr.sh_size) + ")");

llvm/lib/Object/BuildID.cpp

@@ -24,6 +24,24 @@ using namespace llvm::object;
 namespace {
 
 template <typename ELFT> BuildIDRef getBuildID(const ELFFile<ELFT> &Obj) {
+  auto findBuildID = [&Obj](const auto &ShdrOrPhdr,
+                            uint64_t Alignment) -> std::optional<BuildIDRef> {
+    Error Err = Error::success();
+    for (auto N : Obj.notes(ShdrOrPhdr, Err))
+      if (N.getType() == ELF::NT_GNU_BUILD_ID &&
+          N.getName() == ELF::ELF_NOTE_GNU)
+        return N.getDesc(Alignment);
+    consumeError(std::move(Err));
+    return std::nullopt;
+  };
+
+  auto Sections = cantFail(Obj.sections());
+  for (const auto &S : Sections) {
+    if (S.sh_type != ELF::SHT_NOTE)
+      continue;
+    if (std::optional<BuildIDRef> ShdrRes = findBuildID(S, S.sh_addralign))
+      return ShdrRes.value();
+  }
   auto PhdrsOrErr = Obj.program_headers();
   if (!PhdrsOrErr) {
     consumeError(PhdrsOrErr.takeError());
@@ -32,12 +50,8 @@ template <typename ELFT> BuildIDRef getBuildID(const ELFFile<ELFT> &Obj) {
   for (const auto &P : *PhdrsOrErr) {
     if (P.p_type != ELF::PT_NOTE)
       continue;
-    Error Err = Error::success();
-    for (auto N : Obj.notes(P, Err))
-      if (N.getType() == ELF::NT_GNU_BUILD_ID &&
-          N.getName() == ELF::ELF_NOTE_GNU)
-        return N.getDesc(P.p_align);
-    consumeError(std::move(Err));
+    if (std::optional<BuildIDRef> PhdrRes = findBuildID(P, P.p_align))
+      return PhdrRes.value();
   }
   return {};
 }

llvm/test/DebugInfo/symbolize-build-id.test

@@ -21,6 +21,7 @@ Sections:
     Type:    SHT_NOTE
     Flags:   [ SHF_ALLOC ]
     Content: 040000000800000003000000474e5500abb50d82b6bdc861
+    AddressAlign: 4
 ProgramHeaders:
   - Type:     PT_NOTE
     Flags:    [ PF_R ]

llvm/unittests/Object/BuildIDTest.cpp

@@ -0,0 +1,117 @@
+//===- BuildIDTest.cpp - Tests for getBuildID ----------------===//
+//
+// 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/Object/BuildID.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/ObjectYAML/yaml2obj.h"
+#include "llvm/Support/YAMLTraits.h"
+#include "llvm/Testing/Support/Error.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::object;
+
+template <class ELFT>
+static Expected<ELFObjectFile<ELFT>> toBinary(SmallVectorImpl<char> &Storage,
+                                              StringRef Yaml) {
+  raw_svector_ostream OS(Storage);
+  yaml::Input YIn(Yaml);
+  if (!yaml::convertYAML(YIn, OS, [](const Twine &Msg) {}))
+    return createStringError(std::errc::invalid_argument,
+                             "unable to convert YAML");
+  return ELFObjectFile<ELFT>::create(MemoryBufferRef(OS.str(), "dummyELF"));
+}
+
+static StringRef getInvalidNoteELF(bool WithShdr) {
+  static std::string WithSection(R"(
+--- !ELF
+FileHeader:
+  Class:          ELFCLASS64
+  Data:           ELFDATA2LSB
+  Type:           ET_EXEC
+  Machine:        EM_X86_64
+ProgramHeaders:
+  - Type:         PT_NOTE
+    FileSize:     0xffffffffffffff88
+    FirstSec:     .note.gnu.build-id
+    LastSec:      .note.gnu.build-id
+Sections:
+  - Name:         .note.gnu.build-id
+    Type:         SHT_NOTE
+    AddressAlign: 0x04
+    Notes:
+      - Name:     "GNU"
+        Desc:     "abb50d82b6bdc861"
+        Type:     3
+)");
+  static std::string WithoutSection(WithSection + R"(
+  - Type:         SectionHeaderTable
+    NoHeaders:    true
+)");
+  if (WithShdr)
+    return WithSection;
+  return WithoutSection;
+}
+
+// The BuildID can be looked up from a section header, if there is no program header.
+TEST(BuildIDTest, InvalidPhdrFileSizeWithShdrs) {
+  SmallString<0> Storage;
+  Expected<ELFObjectFile<ELF64LE>> ElfOrErr =
+      toBinary<ELF64LE>(Storage, getInvalidNoteELF(true));
+  ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded());
+  BuildIDRef BuildID = getBuildID(&ElfOrErr.get());
+  EXPECT_EQ(
+      StringRef(reinterpret_cast<const char *>(BuildID.data()), BuildID.size()),
+      "\xAB\xB5\x0D\x82\xB6\xBD\xC8\x61");
+}
+
+// The code handles a malformed program header that points at data outside the file.
+TEST(BuildIDTest, InvalidPhdrFileSizeNoShdrs) {
+  SmallString<0> Storage;
+  Expected<ELFObjectFile<ELF64LE>> ElfOrErr =
+      toBinary<ELF64LE>(Storage, getInvalidNoteELF(false));
+  ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded());
+  BuildIDRef BuildID = getBuildID(&ElfOrErr.get());
+  EXPECT_EQ(
+      StringRef(reinterpret_cast<const char *>(BuildID.data()), BuildID.size()),
+      "");
+}
+
+// the code handles a malformed section header that points at data outside the file.
+TEST(BuildIDTest, InvalidSectionHeader) {
+  SmallString<0> Storage;
+  Expected<ELFObjectFile<ELF64LE>> ElfOrErr = toBinary<ELF64LE>(Storage, R"(
+--- !ELF
+FileHeader:
+  Class:          ELFCLASS64
+  Data:           ELFDATA2LSB
+  Type:           ET_EXEC
+  Machine:        EM_X86_64
+ProgramHeaders:
+  - Type:         PT_NOTE
+    FirstSec:     .note.gnu.build-id
+    LastSec:      .note.gnu.build-id
+Sections:
+  - Name:         .note.gnu.build-id
+    Type:         SHT_NOTE
+    AddressAlign: 0x04
+    ShOffset:     0xffffffffffffff88
+    Notes:
+      - Name:     "GNU"
+        Desc:     "abb50d82b6bdc861"
+        Type:     3
+)");
+  ASSERT_THAT_EXPECTED(ElfOrErr, Succeeded());
+  BuildIDRef BuildID = getBuildID(&ElfOrErr.get());
+  EXPECT_EQ(
+      StringRef(reinterpret_cast<const char *>(BuildID.data()), BuildID.size()),
+      "\xAB\xB5\x0D\x82\xB6\xBD\xC8\x61");
+}

llvm/unittests/Object/CMakeLists.txt

@@ -7,6 +7,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_llvm_unittest(ObjectTests
   ArchiveTest.cpp
+  BuildIDTest.cpp
   COFFObjectFileTest.cpp
   DXContainerTest.cpp
   ELFObjectFileTest.cpp