From ac40090583cf8d26c0685028e0361d9d669c9836 Mon Sep 17 00:00:00 2001 From: Agustin Gianni Date: Mon, 5 Dec 2022 15:12:48 +0100 Subject: [PATCH] Kernel: Add the auxiliary vector to the stack size validation This patch validates that the size of the auxiliary vector does not exceed `Process::max_auxiliary_size`. The auxiliary vector is a range of memory in userspace stack where the kernel can pass information to the process that will be created via `Process:do_exec`. The reason the kernel needs to validate its size is that the about to be created process needs to have remaining space on the stack. Previously only `argv` and `envp` were taken into account for the size validation, with this patch, the size of `auxv` is also checked. All three elements contain values that a user (or an attacker) can specify. This patch adds the constant `Process::max_auxiliary_size` which is defined to be one eight of the user-space stack size. This is the approach taken by `Process:max_arguments_size` and `Process::max_environment_size` which are used to check the sizes of `argv` and `envp`. --- Kernel/Process.h | 1 + Kernel/Syscalls/execve.cpp | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Kernel/Process.h b/Kernel/Process.h index 98318fe520..9de6a3f26e 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -466,6 +466,7 @@ public: static constexpr size_t max_arguments_size = Thread::default_userspace_stack_size / 8; static constexpr size_t max_environment_size = Thread::default_userspace_stack_size / 8; + static constexpr size_t max_auxiliary_size = Thread::default_userspace_stack_size / 8; NonnullOwnPtrVector const& arguments() const { return m_arguments; }; NonnullOwnPtrVector const& environment() const { return m_environment; }; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 206b414582..fb659d1606 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -43,18 +43,27 @@ struct LoadResult { static constexpr size_t auxiliary_vector_size = 15; static Array generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, StringView executable_path, Optional const& main_program_fd_allocation); -static bool validate_stack_size(NonnullOwnPtrVector const& arguments, NonnullOwnPtrVector& environment) +static bool validate_stack_size(NonnullOwnPtrVector const& arguments, NonnullOwnPtrVector& environment, Array const& auxiliary) { size_t total_arguments_size = 0; size_t total_environment_size = 0; + size_t total_auxiliary_size = 0; for (auto const& a : arguments) total_arguments_size += a.length() + 1; for (auto const& e : environment) total_environment_size += e.length() + 1; + for (auto const& v : auxiliary) { + if (!v.optional_string.is_empty()) + total_auxiliary_size += round_up_to_power_of_two(v.optional_string.length() + 1, sizeof(FlatPtr)); + + if (v.auxv.a_type == ELF::AuxiliaryValue::Random) + total_auxiliary_size += round_up_to_power_of_two(16, sizeof(FlatPtr)); + } total_arguments_size += sizeof(char*) * (arguments.size() + 1); total_environment_size += sizeof(char*) * (environment.size() + 1); + total_auxiliary_size += sizeof(auxv_t) * auxiliary.size(); if (total_arguments_size > Process::max_arguments_size) return false; @@ -62,7 +71,9 @@ static bool validate_stack_size(NonnullOwnPtrVector const& arguments, N if (total_environment_size > Process::max_environment_size) return false; - // FIXME: This doesn't account for the size of the auxiliary vector + if (total_auxiliary_size > Process::max_auxiliary_size) + return false; + return true; } @@ -476,10 +487,6 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr dbgln_if(EXEC_DEBUG, "do_exec: {}", path); - // FIXME: How much stack space does process startup need? - if (!validate_stack_size(arguments, environment)) - return E2BIG; - auto last_part = path->view().find_last_split_view('/'); auto new_process_name = TRY(KString::try_create(last_part)); @@ -627,6 +634,10 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr auto credentials = this->credentials(); auto auxv = generate_auxiliary_vector(load_result.load_base, load_result.entry_eip, credentials->uid(), credentials->euid(), credentials->gid(), credentials->egid(), path->view(), main_program_fd_allocation); + // FIXME: How much stack space does process startup need? + if (!validate_stack_size(m_arguments, m_environment, auxv)) + return E2BIG; + // NOTE: We create the new stack before disabling interrupts since it will zero-fault // and we don't want to deal with faults after this point. auto new_userspace_sp = TRY(make_userspace_context_for_main_thread(new_main_thread->regs(), *load_result.stack_region.unsafe_ptr(), m_arguments, m_environment, move(auxv)));