Kernel+LibELF: Don't blindly trust ELF symbol offsets in symbolication

It was possible to craft a custom ELF executable that when symbolicated
would cause the kernel to read from user-controlled addresses anywhere
in memory. You could then fetch this memory via /proc/PID/stack

We fix this by making ELFImage hand out StringView rather than raw
const char* for symbol names. In case a symbol offset is outside the
ELF image, you get a null StringView. :^)

Test: Kernel/elf-symbolication-kernel-read-exploit.cpp

4 files changed, 26 insertions, 16 deletions

diff --git a/Libraries/LibELF/ELFImage.cpp b/Libraries/LibELF/ELFImage.cpp
index a7e7df1b77..8beb84d573 100644
--- a/Libraries/LibELF/ELFImage.cpp
+++ b/Libraries/LibELF/ELFImage.cpp
@@ -31,7 +31,7 @@ static const char* object_file_type_to_string(Elf32_Half type)
     }
 }
 
-const char* ELFImage::section_index_to_string(unsigned index) const
+StringView ELFImage::section_index_to_string(unsigned index) const
 {
     if (index == SHN_UNDEF)
         return "Undefined";
@@ -136,20 +136,29 @@ bool ELFImage::parse()
     return true;
 }
 
-const char* ELFImage::section_header_table_string(unsigned offset) const
+StringView ELFImage::table_string(unsigned table_index, unsigned offset) const
 {
-    auto& sh = section_header(header().e_shstrndx);
+    auto& sh = section_header(table_index);
     if (sh.sh_type != SHT_STRTAB)
         return nullptr;
-    return raw_data(sh.sh_offset + offset);
+    size_t computed_offset = sh.sh_offset + offset;
+    if (computed_offset >= m_size) {
+        dbgprintf("SHENANIGANS! ELFImage::table_string() computed offset outside image.\n");
+        return {};
+    }
+    size_t max_length = m_size - computed_offset;
+    size_t length = strnlen(raw_data(sh.sh_offset + offset), max_length);
+    return { raw_data(sh.sh_offset + offset), length };
 }
 
-const char* ELFImage::table_string(unsigned offset) const
+StringView ELFImage::section_header_table_string(unsigned offset) const
 {
-    auto& sh = section_header(m_string_table_section_index);
-    if (sh.sh_type != SHT_STRTAB)
-        return nullptr;
-    return raw_data(sh.sh_offset + offset);
+    return table_string(header().e_shstrndx, offset);
+}
+
+StringView ELFImage::table_string(unsigned offset) const
+{
+    return table_string(m_string_table_section_index, offset);
 }
 
 const char* ELFImage::raw_data(unsigned offset) const
diff --git a/Libraries/LibELF/ELFImage.h b/Libraries/LibELF/ELFImage.h
index c03b8f9991..b1c4fb8bab 100644
--- a/Libraries/LibELF/ELFImage.h
+++ b/Libraries/LibELF/ELFImage.h
@@ -39,7 +39,7 @@ public:
 
         ~Symbol() {}
 
-        const char* name() const { return m_image.table_string(m_sym.st_name); }
+        StringView name() const { return m_image.table_string(m_sym.st_name); }
         unsigned section_index() const { return m_sym.st_shndx; }
         unsigned value() const { return m_sym.st_value; }
         unsigned size() const { return m_sym.st_size; }
@@ -94,7 +94,7 @@ public:
         }
         ~Section() {}
 
-        const char* name() const { return m_image.section_header_table_string(m_section_header.sh_name); }
+        StringView name() const { return m_image.section_header_table_string(m_section_header.sh_name); }
         unsigned type() const { return m_section_header.sh_type; }
         unsigned offset() const { return m_section_header.sh_offset; }
         unsigned size() const { return m_section_header.sh_size; }
@@ -183,9 +183,10 @@ private:
     const Elf32_Ehdr& header() const;
     const Elf32_Shdr& section_header(unsigned) const;
     const Elf32_Phdr& program_header_internal(unsigned) const;
-    const char* table_string(unsigned offset) const;
-    const char* section_header_table_string(unsigned offset) const;
-    const char* section_index_to_string(unsigned index) const;
+    StringView table_string(unsigned offset) const;
+    StringView section_header_table_string(unsigned offset) const;
+    StringView section_index_to_string(unsigned index) const;
+    StringView table_string(unsigned table_index, unsigned offset) const;
 
     const u8* m_buffer { nullptr };
     size_t m_size { 0 };
diff --git a/Libraries/LibELF/ELFLoader.cpp b/Libraries/LibELF/ELFLoader.cpp
index 3797a6875c..ac039a865e 100644
--- a/Libraries/LibELF/ELFLoader.cpp
+++ b/Libraries/LibELF/ELFLoader.cpp
@@ -113,7 +113,7 @@ char* ELFLoader::symbol_ptr(const char* name)
     m_image.for_each_symbol([&](const ELFImage::Symbol symbol) {
         if (symbol.type() != STT_FUNC)
             return IterationDecision::Continue;
-        if (strcmp(symbol.name(), name))
+        if (symbol.name() == name)
             return IterationDecision::Continue;
         if (m_image.is_executable())
             found_ptr = (char*)(size_t)symbol.value();
diff --git a/Libraries/LibELF/ELFLoader.h b/Libraries/LibELF/ELFLoader.h
index fd96a52bab..a8f4de1ee5 100644
--- a/Libraries/LibELF/ELFLoader.h
+++ b/Libraries/LibELF/ELFLoader.h
@@ -51,7 +51,7 @@ private:
 
     struct SortedSymbol {
         u32 address;
-        const char* name;
+        StringView name;
     };
 #ifdef KERNEL
     mutable OwnPtr<Region> m_sorted_symbols_region;