LibELF+Kernel: Validate program headers in Image::parse

This should catch more malformed ELF files earlier than simply
checking the ELF header alone. Also change the API of
validate_program_headers to take the interpreter_path by pointer. This
makes it less awkward to call when we don't care about the interpreter,
and just want the validation.

5 files changed, 30 insertions, 15 deletions

diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp
index b3bfd57010..a1c16ab84e 100644
--- a/Kernel/Syscalls/execve.cpp
+++ b/Kernel/Syscalls/execve.cpp
@@ -402,7 +402,7 @@ KResultOr<NonnullRefPtr<FileDescription>> Process::find_elf_interpreter_for_exec
 
     // Not using KResultOr here because we'll want to do the same thing in userspace in the RTLD
     String interpreter_path;
-    if (!ELF::validate_program_headers(*elf_header, file_size, (u8*)first_page, nread, interpreter_path)) {
+    if (!ELF::validate_program_headers(*elf_header, file_size, (u8*)first_page, nread, &interpreter_path)) {
         dbg() << "exec(" << path << "): File has invalid ELF Program headers";
         return KResult(-ENOEXEC);
     }
@@ -446,7 +446,7 @@ KResultOr<NonnullRefPtr<FileDescription>> Process::find_elf_interpreter_for_exec
 
         // Not using KResultOr here because we'll want to do the same thing in userspace in the RTLD
         String interpreter_interpreter_path;
-        if (!ELF::validate_program_headers(*elf_header, interp_metadata.size, (u8*)first_page, nread, interpreter_interpreter_path)) {
+        if (!ELF::validate_program_headers(*elf_header, interp_metadata.size, (u8*)first_page, nread, &interpreter_interpreter_path)) {
             dbg() << "exec(" << path << "): Interpreter (" << interpreter_description->absolute_path() << ") has invalid ELF Program headers";
             return KResult(-ENOEXEC);
         }
diff --git a/Libraries/LibELF/DynamicLoader.cpp b/Libraries/LibELF/DynamicLoader.cpp
index 9c7fd88969..1a4b4a6d11 100644
--- a/Libraries/LibELF/DynamicLoader.cpp
+++ b/Libraries/LibELF/DynamicLoader.cpp
@@ -84,7 +84,7 @@ DynamicLoader::DynamicLoader(const char* filename, int fd, size_t size)
 
     auto* elf_header = (Elf32_Ehdr*)m_file_mapping;
 
-    if (!validate_elf_header(*elf_header, m_file_size) || !validate_program_headers(*elf_header, m_file_size, (u8*)m_file_mapping, m_file_size, m_program_interpreter)) {
+    if (!validate_elf_header(*elf_header, m_file_size) || !validate_program_headers(*elf_header, m_file_size, (u8*)m_file_mapping, m_file_size, &m_program_interpreter)) {
         m_valid = false;
     }
 }
diff --git a/Libraries/LibELF/Image.cpp b/Libraries/LibELF/Image.cpp
index 9cb0583b4e..d2208234e8 100644
--- a/Libraries/LibELF/Image.cpp
+++ b/Libraries/LibELF/Image.cpp
@@ -151,6 +151,12 @@ bool Image::parse()
         return m_valid = false;
     }
 
+    if (!validate_program_headers(header(), m_size, m_buffer, m_size, nullptr, m_verbose_logging)) {
+        if (m_verbose_logging)
+            dbgputstr("Image::parse(): ELF Program Headers not valid\n");
+        return m_valid = false;
+    }
+
     m_valid = true;
 
     // First locate the string tables.
diff --git a/Libraries/LibELF/Validation.cpp b/Libraries/LibELF/Validation.cpp
index 3f9cd6f2e6..389921d91c 100644
--- a/Libraries/LibELF/Validation.cpp
+++ b/Libraries/LibELF/Validation.cpp
@@ -172,13 +172,14 @@ bool validate_elf_header(const Elf32_Ehdr& elf_header, size_t file_size, bool ve
     return true;
 }
 
-bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, u8* buffer, size_t buffer_size, String& interpreter_path)
+bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, const u8* buffer, size_t buffer_size, String* interpreter_path, bool verbose)
 {
     // Can we actually parse all the program headers in the given buffer?
     size_t end_of_last_program_header = elf_header.e_phoff + (elf_header.e_phnum * elf_header.e_phentsize);
     if (end_of_last_program_header > buffer_size) {
-        dbgprintf("Unable to parse program headers from buffer, buffer too small! Buffer size: %zu, End of program headers %zu\n",
-            buffer_size, end_of_last_program_header);
+        if (verbose)
+            dbgprintf("Unable to parse program headers from buffer, buffer too small! Buffer size: %zu, End of program headers %zu\n",
+                buffer_size, end_of_last_program_header);
         return false;
     }
 
@@ -195,15 +196,18 @@ bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, u8
         switch (program_header.p_type) {
         case PT_INTERP:
             if (ET_DYN != elf_header.e_type) {
-                dbgprintf("Found PT_INTERP header (%zu) in non-DYN ELF object! What? We can't handle this!\n", header_index);
+                if (verbose)
+                    dbgprintf("Found PT_INTERP header (%zu) in non-DYN ELF object! What? We can't handle this!\n", header_index);
                 return false;
             }
             // We checked above that file_size was >= buffer size. We only care about buffer size anyway, we're trying to read this!
             if (program_header.p_offset + program_header.p_filesz > buffer_size) {
-                dbgprintf("Found PT_INTERP header (%zu), but the .interp section was not within our buffer :( Your program will not be loaded today.\n", header_index);
+                if (verbose)
+                    dbgprintf("Found PT_INTERP header (%zu), but the .interp section was not within our buffer :( Your program will not be loaded today.\n", header_index);
                 return false;
             }
-            interpreter_path = String((const char*)&buffer[program_header.p_offset], program_header.p_filesz - 1);
+            if (interpreter_path)
+                *interpreter_path = String((const char*)&buffer[program_header.p_offset], program_header.p_filesz - 1);
             break;
         case PT_LOAD:
         case PT_DYNAMIC:
@@ -211,28 +215,33 @@ bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, u8
         case PT_PHDR:
         case PT_TLS:
             if (program_header.p_offset + program_header.p_filesz > file_size) {
-                dbgprintf("SHENANIGANS! Program header %zu segment leaks beyond end of file!\n", header_index);
+                if (verbose)
+                    dbgprintf("SHENANIGANS! Program header %zu segment leaks beyond end of file!\n", header_index);
                 return false;
             }
             if ((program_header.p_flags & PF_X) && (program_header.p_flags & PF_W)) {
-                dbgprintf("SHENANIGANS! Program header %zu segment is marked write and execute\n", header_index);
+                if (verbose)
+                    dbgprintf("SHENANIGANS! Program header %zu segment is marked write and execute\n", header_index);
                 return false;
             }
             break;
         case PT_GNU_STACK:
             if (program_header.p_flags & PF_X) {
-                dbgprintf("Possible shenanigans! Validating an ELF with executable stack.\n");
+                if (verbose)
+                    dbgprintf("Possible shenanigans! Validating an ELF with executable stack.\n");
             }
             break;
         case PT_GNU_RELRO:
             if ((program_header.p_flags & PF_X) && (program_header.p_flags & PF_W)) {
-                dbgprintf("SHENANIGANS! Program header %zu segment is marked write and execute\n", header_index);
+                if (verbose)
+                    dbgprintf("SHENANIGANS! Program header %zu segment is marked write and execute\n", header_index);
                 return false;
             }
             break;
         default:
             // Not handling other program header types in other code so... let's not surprise them
-            dbgprintf("Found program header (%zu) of unrecognized type %x!\n", header_index, program_header.p_type);
+            if (verbose)
+                dbgprintf("Found program header (%zu) of unrecognized type %x!\n", header_index, program_header.p_type);
             return false;
         }
     }
diff --git a/Libraries/LibELF/Validation.h b/Libraries/LibELF/Validation.h
index a14ea3b87b..fe49c80b82 100644
--- a/Libraries/LibELF/Validation.h
+++ b/Libraries/LibELF/Validation.h
@@ -31,6 +31,6 @@
 namespace ELF {
 
 bool validate_elf_header(const Elf32_Ehdr& elf_header, size_t file_size, bool verbose = true);
-bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, u8* buffer, size_t buffer_size, String& interpreter_path);
+bool validate_program_headers(const Elf32_Ehdr& elf_header, size_t file_size, const u8* buffer, size_t buffer_size, String* interpreter_path, bool verbose = true);
 
 } // end namespace ELF