Merge pull request #14744 from acts-1631/security/elf-reader-bounds

Core/Boot: validate standalone ELF input ranges
2026-08-03 20:24:14 +02:00 · 2026-07-21 14:58:38 +12:00
parent b24783f84d 49453c045c
commit 00626b263f
2 changed files with 112 additions and 29 deletions
@@ -3,6 +3,7 @@

 #include "Core/Boot/ElfReader.h"

+#include <cstring>
 #include <string>
 #include <utility>

@@ -70,62 +71,113 @@ static void byteswapSection(Elf32_Shdr& sec)

 ElfReader::ElfReader(std::vector<u8> buffer) : BootExecutableReader(std::move(buffer))
 {
-  Initialize(m_bytes.data());
+  m_is_valid = Initialize();
 }

 ElfReader::ElfReader(File::IOFile file) : BootExecutableReader(std::move(file))
 {
-  Initialize(m_bytes.data());
+  m_is_valid = Initialize();
 }

 ElfReader::ElfReader(const std::string& filename) : BootExecutableReader(filename)
 {
-  Initialize(m_bytes.data());
+  m_is_valid = Initialize();
 }

 ElfReader::~ElfReader() = default;

-void ElfReader::Initialize(u8* ptr)
+bool ElfReader::Initialize()
 {
-  base = (char*)ptr;
-  base32 = (u32*)ptr;
-  header = (Elf32_Ehdr*)ptr;
+  if (m_bytes.size() < sizeof(Elf32_Ehdr))
+  {
+    ERROR_LOG_FMT(BOOT, "ELF file is too small.");
+    return false;
+  }
+
+  base = reinterpret_cast<char*>(m_bytes.data());
+  base32 = reinterpret_cast<u32*>(m_bytes.data());
+  header = reinterpret_cast<Elf32_Ehdr*>(m_bytes.data());
+  if (header->e_ident[EI_MAG0] != ELFMAG0 || header->e_ident[EI_MAG1] != ELFMAG1 ||
+      header->e_ident[EI_MAG2] != ELFMAG2 || header->e_ident[EI_MAG3] != ELFMAG3 ||
+      header->e_ident[EI_CLASS] != ELFCLASS32 || header->e_ident[EI_DATA] != ELFDATA2MSB)
+  {
+    ERROR_LOG_FMT(BOOT, "Invalid ELF header.");
+    return false;
+  }
+
  byteswapHeader(*header);

-  segments = (Elf32_Phdr*)(base + header->e_phoff);
-  sections = (Elf32_Shdr*)(base + header->e_shoff);
+  const auto is_range_valid = [this](size_t offset, size_t size) {
+    return offset <= m_bytes.size() && size <= m_bytes.size() - offset;
+  };
+  if (header->e_ehsize != sizeof(Elf32_Ehdr) ||
+      (header->e_phnum != 0 && header->e_phentsize != sizeof(Elf32_Phdr)) ||
+      (header->e_shnum != 0 && header->e_shentsize != sizeof(Elf32_Shdr)) ||
+      !is_range_valid(header->e_phoff, sizeof(Elf32_Phdr) * header->e_phnum) ||
+      !is_range_valid(header->e_shoff, sizeof(Elf32_Shdr) * header->e_shnum) ||
+      (header->e_shstrndx != SHN_UNDEF && header->e_shstrndx >= header->e_shnum))
+  {
+    ERROR_LOG_FMT(BOOT, "Invalid ELF header table.");
+    return false;
+  }
+
+  segments = reinterpret_cast<Elf32_Phdr*>(base + header->e_phoff);
+  sections = reinterpret_cast<Elf32_Shdr*>(base + header->e_shoff);

  for (int i = 0; i < GetNumSegments(); i++)
  {
    byteswapSegment(segments[i]);
+    if (!is_range_valid(segments[i].p_offset, segments[i].p_filesz) ||
+        segments[i].p_filesz > segments[i].p_memsz)
+    {
+      ERROR_LOG_FMT(BOOT, "Invalid ELF program header {}.", i);
+      return false;
+    }
  }

  for (int i = 0; i < GetNumSections(); i++)
  {
    byteswapSection(sections[i]);
+    if (sections[i].sh_type != SHT_NOBITS &&
+        !is_range_valid(sections[i].sh_offset, sections[i].sh_size))
+    {
+      ERROR_LOG_FMT(BOOT, "Invalid ELF section header {}.", i);
+      return false;
+    }
  }
  entryPoint = header->e_entry;

  bRelocate = (header->e_type != ET_EXEC);
+  return true;
 }

 const char* ElfReader::GetSectionName(int section) const
 {
-  if (sections[section].sh_type == SHT_NULL)
+  if (!m_is_valid || section < 0 || section >= header->e_shnum ||
+      sections[section].sh_type == SHT_NULL)
+  {
    return nullptr;
+  }

-  int nameOffset = sections[section].sh_name;
-  char* ptr = (char*)GetSectionDataPtr(header->e_shstrndx);
+  const Elf32_Shdr& string_section = sections[header->e_shstrndx];
+  const size_t name_offset = sections[section].sh_name;
+  const char* const ptr = reinterpret_cast<const char*>(GetSectionDataPtr(header->e_shstrndx));

-  if (ptr)
-    return ptr + nameOffset;
-  else
+  if (!ptr || name_offset >= string_section.sh_size ||
+      !std::memchr(ptr + name_offset, '\0', string_section.sh_size - name_offset))
+  {
    return nullptr;
+  }
+
+  return ptr + name_offset;
 }

 // This is just a simple elf loader, good enough to load elfs generated by devkitPPC
 bool ElfReader::LoadIntoMemory(Core::System& system, bool only_in_mem1) const
 {
+  if (!m_is_valid)
+    return false;
+
  INFO_LOG_FMT(BOOT, "String section: {}", header->e_shstrndx);

  if (bRelocate)
@@ -183,15 +235,36 @@ SectionID ElfReader::GetSectionByName(const char* name, int firstSection) const
 bool ElfReader::LoadSymbols(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_symbol_db,
                            const std::string& filename) const
 {
+  if (!m_is_valid)
+    return false;
+
  bool hasSymbols = false;
  SectionID sec = GetSectionByName(".symtab");
  if (sec != -1)
  {
-    int stringSection = sections[sec].sh_link;
-    const char* stringBase = (const char*)GetSectionDataPtr(stringSection);
+    const u32 string_section_index = sections[sec].sh_link;
+    if (string_section_index >= header->e_shnum)
+    {
+      ERROR_LOG_FMT(BOOT, "Invalid ELF symbol string table.");
+      return false;
+    }
+
+    const Elf32_Shdr& string_section = sections[string_section_index];
+    const char* stringBase = (const char*)GetSectionDataPtr(string_section_index);
+    if (!stringBase)
+    {
+      ERROR_LOG_FMT(BOOT, "ELF symbol string table has no data.");
+      return false;
+    }

    // We have a symbol table!
    Elf32_Sym* symtab = (Elf32_Sym*)(GetSectionDataPtr(sec));
+    if (!symtab)
+    {
+      ERROR_LOG_FMT(BOOT, "ELF symbol table has no data.");
+      return false;
+    }
+
    int numSymbols = sections[sec].sh_size / sizeof(Elf32_Sym);
    for (int sym = 0; sym < numSymbols; sym++)
    {
@@ -203,7 +276,14 @@ bool ElfReader::LoadSymbols(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_
      int type = symtab[sym].st_info & 0xF;
      int sectionIndex = Common::swap16(symtab[sym].st_shndx);
      int value = Common::swap32(symtab[sym].st_value);
-      const char* name = stringBase + Common::swap32(symtab[sym].st_name);
+      const size_t name_offset = Common::swap32(symtab[sym].st_name);
+      if (name_offset >= string_section.sh_size ||
+          !std::memchr(stringBase + name_offset, '\0', string_section.sh_size - name_offset))
+      {
+        ERROR_LOG_FMT(BOOT, "Invalid ELF symbol name {}.", sym);
+        return false;
+      }
+      const char* name = stringBase + name_offset;
      if (bRelocate)
        value += sectionAddrs[sectionIndex];

@@ -229,6 +309,9 @@ bool ElfReader::LoadSymbols(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_

 bool ElfReader::IsWii() const
 {
+  if (!m_is_valid)
+    return false;
+
  // Use the same method as the DOL loader uses: search for mfspr from HID4,
  // which should only be used in Wii ELFs.
  //
@@ -38,8 +38,7 @@ public:
  bool LoadIntoMemory(Core::System& system, bool only_in_mem1 = false) const override;
  bool LoadSymbols(const Core::CPUThreadGuard& guard, PPCSymbolDB& ppc_symbol_db,
                   const std::string& filename) const override;
-  // TODO: actually check for validity.
-  bool IsValid() const override { return true; }
+  bool IsValid() const override { return m_is_valid; }
  bool IsWii() const override;

  int GetNumSegments() const { return (int)(header->e_phnum); }
@@ -65,16 +64,17 @@ public:
  bool DidRelocate() const { return bRelocate; }

 private:
-  void Initialize(u8* bytes);
+  bool Initialize();

-  char* base;
-  u32* base32;
+  char* base = nullptr;
+  u32* base32 = nullptr;

-  Elf32_Ehdr* header;
-  Elf32_Phdr* segments;
-  Elf32_Shdr* sections;
+  Elf32_Ehdr* header = nullptr;
+  Elf32_Phdr* segments = nullptr;
+  Elf32_Shdr* sections = nullptr;

-  u32* sectionAddrs;
-  bool bRelocate;
-  u32 entryPoint;
+  u32* sectionAddrs = nullptr;
+  bool bRelocate = false;
+  u32 entryPoint = 0;
+  bool m_is_valid = false;
 };