diff options
author | Andreas Kling <kling@serenityos.org> | 2020-12-20 18:35:29 +0100 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2020-12-20 18:49:18 +0100 |
commit | 9bf02c32c040d09019bb6b459844690825694fa7 (patch) | |
tree | 2b534a3e28dee341e9fc2373c872ce55a295803c | |
parent | 5505159a94c226d0c8a5e82a163a6d37a9404c57 (diff) | |
download | serenity-9bf02c32c040d09019bb6b459844690825694fa7.zip |
Kernel: Activate SUID/SGID credentials earlier in sys$execve()
Switch on the new credentials before loading the new executable into
memory. This ensures that attempts to ptrace() the program from an
unprivileged process will fail.
This covers one bug that was exploited in the 2020 HXP CTF:
https://hxp.io/blog/79/hxp-CTF-2020-wisdom2/
Thanks to yyyyyyy for finding the bug! :^)
-rw-r--r-- | Kernel/Syscalls/execve.cpp | 39 |
1 files changed, 30 insertions, 9 deletions
diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 2561e1d515..4afadb1576 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -237,12 +237,42 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve dbg() << "Process " << pid().value() << " exec: PD=" << m_page_directory.ptr() << " created"; #endif + // NOTE: We switch credentials before altering the memory layout of the process. + // This ensures that ptrace access control takes the right credentials into account. + + // FIXME: This still feels rickety. Perhaps it would be better to simply block ptrace + // clients until we're ready to be traced? Or reject them with EPERM? + + auto main_program_metadata = main_program_description->metadata(); + + auto old_euid = m_euid; + auto old_suid = m_suid; + auto old_egid = m_egid; + auto old_sgid = m_sgid; + + ArmedScopeGuard cred_restore_guard = [&] { + m_euid = old_euid; + m_suid = old_suid; + m_egid = old_egid; + m_sgid = old_sgid; + }; + + if (!(main_program_description->custody()->mount_flags() & MS_NOSUID)) { + if (main_program_metadata.is_setuid()) + m_euid = m_suid = main_program_metadata.uid; + if (main_program_metadata.is_setgid()) + m_egid = m_sgid = main_program_metadata.gid; + } + int load_rc = load(main_program_description, interpreter_description); if (load_rc) { klog() << "do_exec: Failed to load main program or interpreter"; return load_rc; } + // We can commit to the new credentials at this point. + cred_restore_guard.disarm(); + kill_threads_except_self(); #ifdef EXEC_DEBUG @@ -257,15 +287,6 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve m_veil_state = VeilState::None; m_unveiled_paths.clear(); - auto main_program_metadata = main_program_description->metadata(); - - if (!(main_program_description->custody()->mount_flags() & MS_NOSUID)) { - if (main_program_metadata.is_setuid()) - m_euid = m_suid = main_program_metadata.uid; - if (main_program_metadata.is_setgid()) - m_egid = m_sgid = main_program_metadata.gid; - } - current_thread->set_default_signal_dispositions(); current_thread->clear_signals(); |