diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 431d0d21d8..9b8bd908e3 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -49,6 +49,7 @@ //#define DEBUG_IO //#define TASK_DEBUG //#define FORK_DEBUG +//#define EXEC_DEBUG //#define SIGNAL_DEBUG //#define SHARED_BUFFER_DEBUG @@ -655,7 +656,7 @@ int Process::do_exec(String path, Vector arguments, Vector envir // Okay, here comes the sleight of hand, pay close attention.. auto old_regions = move(m_regions); m_regions.append(move(executable_region)); - loader = make(region->vaddr().as_ptr()); + loader = make(region->vaddr().as_ptr(), metadata.size); loader->map_section_hook = [&](VirtualAddress vaddr, size_t size, size_t alignment, size_t offset_in_image, bool is_readable, bool is_writable, bool is_executable, const String& name) -> u8* { ASSERT(size); ASSERT(alignment == PAGE_SIZE); @@ -703,6 +704,11 @@ int Process::do_exec(String path, Vector arguments, Vector envir // NOTE: At this point, we've committed to the new executable. entry_eip = loader->entry().get(); + +#ifdef EXEC_DEBUG + kprintf("Memory layout after ELF load:"); + dump_regions(); +#endif } region->set_user_accessible(false); @@ -3818,8 +3824,7 @@ int Process::sys$module_load(const char* user_path, size_t path_length) memcpy(storage.data(), payload.data(), payload.size()); payload.clear(); - // FIXME: ELFImage should really be taking a size argument as well... - auto elf_image = make(storage.data()); + auto elf_image = make(storage.data(), storage.size()); if (!elf_image->parse()) return -ENOEXEC; diff --git a/Libraries/LibELF/ELFDynamicLoader.cpp b/Libraries/LibELF/ELFDynamicLoader.cpp index ef1e0e71ed..2a6dcb0780 100644 --- a/Libraries/LibELF/ELFDynamicLoader.cpp +++ b/Libraries/LibELF/ELFDynamicLoader.cpp @@ -56,7 +56,7 @@ void* ELFDynamicLoader::symbol_for_name(const char* name) bool ELFDynamicLoader::load_from_image(unsigned flags) { - ELFImage elf_image((u8*)m_file_mapping); + ELFImage elf_image((u8*)m_file_mapping, m_file_size); m_valid = elf_image.is_valid() && elf_image.is_dynamic(); diff --git a/Libraries/LibELF/ELFImage.cpp b/Libraries/LibELF/ELFImage.cpp index cb93e2abc3..33ac0ff426 100644 --- a/Libraries/LibELF/ELFImage.cpp +++ b/Libraries/LibELF/ELFImage.cpp @@ -2,8 +2,9 @@ #include #include -ELFImage::ELFImage(const u8* buffer) +ELFImage::ELFImage(const u8* buffer, size_t size) : m_buffer(buffer) + , m_size(size) { m_valid = parse(); } @@ -59,6 +60,8 @@ void ELFImage::dump() const dbgprintf(" entry: %x\n", header().e_entry); dbgprintf(" shoff: %u\n", header().e_shoff); dbgprintf(" shnum: %u\n", header().e_shnum); + dbgprintf(" phoff: %u\n", header().e_phoff); + dbgprintf(" phnum: %u\n", header().e_phnum); dbgprintf(" shstrndx: %u\n", header().e_shstrndx); for (unsigned i = 0; i < header().e_shnum; ++i) { diff --git a/Libraries/LibELF/ELFImage.h b/Libraries/LibELF/ELFImage.h index 88b436dd38..816c8e0682 100644 --- a/Libraries/LibELF/ELFImage.h +++ b/Libraries/LibELF/ELFImage.h @@ -8,12 +8,21 @@ class ELFImage { public: - explicit ELFImage(const u8*); + explicit ELFImage(const u8*, size_t); ~ELFImage(); void dump() const; bool is_valid() const { return m_valid; } bool parse(); + bool is_within_image(const void* address, size_t size) + { + if (address < m_buffer) + return false; + if (((const u8*)address + size) > m_buffer + m_size) + return false; + return true; + } + class Section; class RelocationSection; class Symbol; @@ -190,6 +199,7 @@ private: const char* section_index_to_string(unsigned index) const; const u8* m_buffer { nullptr }; + size_t m_size { 0 }; HashMap m_sections; bool m_valid { false }; unsigned m_symbol_table_section_index { 0 }; diff --git a/Libraries/LibELF/ELFLoader.cpp b/Libraries/LibELF/ELFLoader.cpp index ab8d38fba3..3c89246774 100644 --- a/Libraries/LibELF/ELFLoader.cpp +++ b/Libraries/LibELF/ELFLoader.cpp @@ -9,8 +9,8 @@ //#define ELFLOADER_DEBUG -ELFLoader::ELFLoader(const u8* buffer) - : m_image(buffer) +ELFLoader::ELFLoader(const u8* buffer, size_t size) + : m_image(buffer, size) { } @@ -43,6 +43,11 @@ bool ELFLoader::layout() failed = true; return; } + if (!m_image.is_within_image(program_header.raw_data(), program_header.size_in_image())) { + dbg() << "Shenanigans! ELF PT_TLS header sneaks outside of executable."; + failed = true; + return; + } memcpy(tls_image, program_header.raw_data(), program_header.size_in_image()); #endif return; @@ -65,6 +70,11 @@ bool ELFLoader::layout() failed = true; return; } + if (!m_image.is_within_image(program_header.raw_data(), program_header.size_in_image())) { + dbg() << "Shenanigans! Writable ELF PT_LOAD header sneaks outside of executable."; + failed = true; + return; + } memcpy(program_header.vaddr().as_ptr(), program_header.raw_data(), program_header.size_in_image()); } else { auto* mapped_section = map_section_hook( diff --git a/Libraries/LibELF/ELFLoader.h b/Libraries/LibELF/ELFLoader.h index 6d80fd31b1..fd96a52bab 100644 --- a/Libraries/LibELF/ELFLoader.h +++ b/Libraries/LibELF/ELFLoader.h @@ -13,7 +13,7 @@ class Region; class ELFLoader { public: - explicit ELFLoader(const u8*); + explicit ELFLoader(const u8*, size_t); ~ELFLoader(); bool load();