diff options
author | Andreas Kling <kling@serenityos.org> | 2021-02-19 09:41:35 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-02-19 09:46:36 +0100 |
commit | 37d8faf1b45a88ca195175f63a2bad374fca3e6e (patch) | |
tree | 7dc8fd836a09244cce28a5d4dcd8300dd948ed77 | |
parent | 7142562310e631156d1f64aff22f068ae2c48a5e (diff) | |
download | serenity-37d8faf1b45a88ca195175f63a2bad374fca3e6e.zip |
ProcFS: Fix /proc/PID/* hardening bypass
This enabled trivial ASLR bypass for non-dumpable programs by simply
opening /proc/PID/vm before exec'ing.
We now hold the target process's ptrace lock across the refresh/write
operations, and deny access if the process is non-dumpable. The lock
is necessary to prevent a TOCTOU race on Process::is_dumpable() while
the target is exec'ing.
Fixes #5270.
-rw-r--r-- | Kernel/FileSystem/ProcFS.cpp | 38 | ||||
-rw-r--r-- | Kernel/FileSystem/ProcFS.h | 2 |
2 files changed, 40 insertions, 0 deletions
diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index e98154094b..14789c8d98 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -28,6 +28,7 @@ #include <AK/JsonObject.h> #include <AK/JsonObjectSerializer.h> #include <AK/JsonValue.h> +#include <AK/ScopeGuard.h> #include <Kernel/Arch/i386/CPU.h> #include <Kernel/Arch/i386/ProcessorInfo.h> #include <Kernel/CommandLine.h> @@ -1048,11 +1049,32 @@ ProcFSInode::~ProcFSInode() fs().m_inodes.remove(it); } +RefPtr<Process> ProcFSInode::process() const +{ + return Process::from_pid(to_pid(identifier())); +} + KResult ProcFSInode::refresh_data(FileDescription& description) const { if (Kernel::is_directory(identifier())) return KSuccess; + // For process-specific inodes, hold the process's ptrace lock across refresh + // and refuse to load data if the process is not dumpable. + // Without this, files opened before a process went non-dumpable could still be used for dumping. + auto process = this->process(); + if (process) { + process->ptrace_lock().lock(); + if (!process->is_dumpable()) { + process->ptrace_lock().unlock(); + return EPERM; + } + } + ScopeGuard guard = [&] { + if (process) + process->ptrace_lock().unlock(); + }; + auto& cached_data = description.data(); auto* directory_entry = fs().get_directory_entry(identifier()); @@ -1440,6 +1462,22 @@ void ProcFSInode::flush_metadata() ssize_t ProcFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& buffer, FileDescription*) { + // For process-specific inodes, hold the process's ptrace lock across the write + // and refuse to write at all data if the process is not dumpable. + // Without this, files opened before a process went non-dumpable could still be used for dumping. + auto process = this->process(); + if (process) { + process->ptrace_lock().lock(); + if (!process->is_dumpable()) { + process->ptrace_lock().unlock(); + return EPERM; + } + } + ScopeGuard guard = [&] { + if (process) + process->ptrace_lock().unlock(); + }; + auto result = prepare_to_write_data(); if (result.is_error()) return result; diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index dd56fb448f..d5cf67939f 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -114,6 +114,8 @@ private: KResult refresh_data(FileDescription&) const; + RefPtr<Process> process() const; + ProcFS& fs() { return static_cast<ProcFS&>(Inode::fs()); } const ProcFS& fs() const { return static_cast<const ProcFS&>(Inode::fs()); } ProcFSInode(ProcFS&, InodeIndex); |