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

Core/Boot: validate standalone ELF input ranges

2 files changed, 112 insertions, 29 deletions

diff --git a/Source/Core/Core/Boot/ElfReader.cpp b/Source/Core/Core/Boot/ElfReader.cpp
index f667387ab3..8cdc1770a5 100644
--- a/Source/Core/Core/Boot/ElfReader.cpp
+++ b/Source/Core/Core/Boot/ElfReader.cpp
@@ -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.
   //
diff --git a/Source/Core/Core/Boot/ElfReader.h b/Source/Core/Core/Boot/ElfReader.h
index 83fd878012..cb6c895496 100644
--- a/Source/Core/Core/Boot/ElfReader.h
+++ b/Source/Core/Core/Boot/ElfReader.h
@@ -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;
 };