summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndreas Kling <kling@serenityos.org>2021-02-19 09:41:35 +0100
committerAndreas Kling <kling@serenityos.org>2021-02-19 09:46:36 +0100
commit37d8faf1b45a88ca195175f63a2bad374fca3e6e (patch)
tree7dc8fd836a09244cce28a5d4dcd8300dd948ed77
parent7142562310e631156d1f64aff22f068ae2c48a5e (diff)
downloadserenity-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.cpp38
-rw-r--r--Kernel/FileSystem/ProcFS.h2
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);