From 190e0e155130e09dc9c9b7a0989cdb0ef8c55a7d Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 10 Jan 2021 11:27:02 +0100
Subject: [PATCH] Kernel+SystemServer+CrashDaemon: Better control where we put
 core dumps

SystemServer now creates the /tmp/coredump and /tmp/profiler_coredumps
directories at startup, ensuring that they are owned by root, and with
basic 0755 permissions.

The kernel will also now refuse to put core dumps in a directory that
doesn't fulfill the following criteria:

- Owned by 0:0
- Directory with sticky bit not set
- 0755 permissions

Fixes #4435
Fixes #4850
---
 Kernel/CoreDump.cpp            | 20 ++++++++++++--------
 Services/CrashDaemon/main.cpp  |  4 +---
 Services/SystemServer/main.cpp | 26 ++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/Kernel/CoreDump.cpp b/Kernel/CoreDump.cpp
index e8f3f6f5cd25d0..6c79afe3654daf 100644
--- a/Kernel/CoreDump.cpp
+++ b/Kernel/CoreDump.cpp
@@ -69,23 +69,27 @@ RefPtr<FileDescription> CoreDump::create_target_file(const Process& process, con
 {
     LexicalPath lexical_path(output_path);
     auto output_directory = lexical_path.dirname();
-    if (VFS::the().open_directory(output_directory, VFS::the().root_custody()).is_error()) {
-        auto res = VFS::the().mkdir(output_directory, 0777, VFS::the().root_custody());
-        if (res.is_error())
-            return nullptr;
+    auto dump_directory = VFS::the().open_directory(output_directory, VFS::the().root_custody());
+    if (dump_directory.is_error()) {
+        dbgln("Can't find directory '{}' for core dump", output_directory);
+        return nullptr;
     }
-    auto tmp_dir = VFS::the().open_directory(output_directory, VFS::the().root_custody());
-    if (tmp_dir.is_error())
+    auto dump_directory_metadata = dump_directory.value()->inode().metadata();
+    if (dump_directory_metadata.uid != 0 || dump_directory_metadata.gid != 0 || dump_directory_metadata.mode != 040755) {
+        dbgln("Refusing to put core dump in sketchy directory '{}'", output_directory);
         return nullptr;
+    }
     auto fd_or_error = VFS::the().open(
         lexical_path.basename(),
         O_CREAT | O_WRONLY | O_EXCL,
         0, // We will enable reading from userspace when we finish generating the coredump file
-        *tmp_dir.value(),
+        *dump_directory.value(),
         UidAndGid { process.uid(), process.gid() });
 
-    if (fd_or_error.is_error())
+    if (fd_or_error.is_error()) {
+        dbgln("Failed to open core dump '{}' for writing", output_path);
         return nullptr;
+    }
 
     return fd_or_error.value();
 }
diff --git a/Services/CrashDaemon/main.cpp b/Services/CrashDaemon/main.cpp
index 857415bdd6c326..b16d412074aae6 100644
--- a/Services/CrashDaemon/main.cpp
+++ b/Services/CrashDaemon/main.cpp
@@ -74,9 +74,7 @@ static void launch_crash_reporter(const String& coredump_path)
 
 int main()
 {
-    static constexpr const char* coredumps_dir = "/tmp/coredump";
-    mkdir(coredumps_dir, 0777);
-    Core::DirectoryWatcher watcher { coredumps_dir };
+    Core::DirectoryWatcher watcher { "/tmp/coredump" };
     while (true) {
         auto event = watcher.wait_for_event();
         ASSERT(event.has_value());
diff --git a/Services/SystemServer/main.cpp b/Services/SystemServer/main.cpp
index 19cf7b2895c852..5233922f2e322e 100644
--- a/Services/SystemServer/main.cpp
+++ b/Services/SystemServer/main.cpp
@@ -192,6 +192,30 @@ static void create_tmp_rpc_directory()
     umask(old_umask);
 }
 
+static void create_tmp_coredump_directory()
+{
+    dbgln("Creating /tmp/coredump directory");
+    auto old_umask = umask(0);
+    auto rc = mkdir("/tmp/coredump", 0755);
+    if (rc < 0) {
+        perror("mkdir(/tmp/coredump)");
+        ASSERT_NOT_REACHED();
+    }
+    umask(old_umask);
+}
+
+static void create_tmp_profiler_coredumps_directory()
+{
+    dbgln("Creating /tmp/profiler_coredumps directory");
+    auto old_umask = umask(0);
+    auto rc = mkdir("/tmp/profiler_coredumps", 0755);
+    if (rc < 0) {
+        perror("mkdir(/tmp/profiler_coredumps)");
+        ASSERT_NOT_REACHED();
+    }
+    umask(old_umask);
+}
+
 int main(int, char**)
 {
     prepare_devfs();
@@ -203,6 +227,8 @@ int main(int, char**)
 
     mount_all_filesystems();
     create_tmp_rpc_directory();
+    create_tmp_coredump_directory();
+    create_tmp_profiler_coredumps_directory();
     parse_boot_mode();
 
     Core::EventLoop event_loop;