diff --git a/Base/usr/share/man/man7/Mitigations.md b/Base/usr/share/man/man7/Mitigations.md index e7700b4ded..299dea110b 100644 --- a/Base/usr/share/man/man7/Mitigations.md +++ b/Base/usr/share/man/man7/Mitigations.md @@ -105,6 +105,7 @@ Special restrictions on filesystem also apply: - Write access is forbidden to kernel variables (which are located in `/sys/kernel/variables`). - Open access is forbidden to all device nodes except for `/dev/full`, `/dev/null`, `/dev/zero`, `/dev/random` and various other TTY/PTY devices (not including Kernel virtual consoles). +- Executing SUID binaries is forbidden. It was first added in the following [commit](https://github.com/SerenityOS/serenity/commit/5e062414c11df31ed595c363990005eef00fa263), for kernel support, and the following commits added basic userspace utilities: diff --git a/Kernel/Process.h b/Kernel/Process.h index bf9d2d0c0c..c897d4f962 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -118,6 +118,7 @@ class Process final // FIXME: This should be a NonnullRefPtr RefPtr credentials; bool dumpable { false }; + bool executable_is_setid { false }; Atomic has_promises { false }; Atomic promises { 0 }; Atomic has_execpromises { false }; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index d201b22616..4f1e06d149 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -472,6 +472,15 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr { VERIFY(is_user_process()); VERIFY(!Processor::in_critical()); + auto main_program_metadata = main_program_description->metadata(); + // NOTE: Don't allow running SUID binaries at all if we are in a jail. + TRY(Process::current().jail().with([&](auto& my_jail) -> ErrorOr { + if (my_jail && (main_program_metadata.is_setuid() || main_program_metadata.is_setgid())) { + return Error::from_errno(EPERM); + } + return {}; + })); + // Although we *could* handle a pseudo_path here, trying to execute something that doesn't have // a custody (e.g. BlockDevice or RandomDevice) is pretty suspicious anyway. auto path = TRY(main_program_description->original_absolute_path()); @@ -507,8 +516,6 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr bool executable_is_setid = false; if (!(main_program_description->custody()->mount_flags() & MS_NOSUID)) { - auto main_program_metadata = main_program_description->metadata(); - auto new_euid = old_credentials->euid(); auto new_egid = old_credentials->egid(); auto new_suid = old_credentials->suid(); @@ -551,6 +558,7 @@ ErrorOr Process::do_exec(NonnullLockRefPtr main_progr with_mutable_protected_data([&](auto& protected_data) { protected_data.credentials = move(new_credentials); protected_data.dumpable = !executable_is_setid; + protected_data.executable_is_setid = executable_is_setid; }); // We make sure to enter the new address space before destroying the old one. diff --git a/Kernel/Syscalls/jail.cpp b/Kernel/Syscalls/jail.cpp index 68497bf515..0f5ac4e16c 100644 --- a/Kernel/Syscalls/jail.cpp +++ b/Kernel/Syscalls/jail.cpp @@ -45,6 +45,17 @@ ErrorOr Process::sys$jail_attach(Userspace ErrorOr { + if (protected_data.executable_is_setid) + return EPERM; + return {}; + })); + auto params = TRY(copy_typed_from_user(user_params)); return m_attached_jail.with([&](auto& my_jail) -> ErrorOr { // Note: If we are already in a jail, don't let the process escape it even if