From d44e2c9ad9469ed07308870fcf7956769a5ccec4 Mon Sep 17 00:00:00 2001 From: Jesse Buhagiar Date: Sat, 17 Apr 2021 00:55:05 +1000 Subject: [PATCH] Userland: Check sudoers file perms and owner in pls As per comment found in #6319 by @bcoles, `pls` should check the permissions and owner of the sudoers file to ensure that it hasn't been compromised. --- Base/etc/{sudoers => plsusers} | 2 +- Base/usr/share/man/man8/pls.md | 4 +- Documentation/BuildInstructions.md | 4 +- Meta/build-root-filesystem.sh | 4 + Userland/Utilities/pls.cpp | 171 +++++++++++++++-------------- 5 files changed, 99 insertions(+), 86 deletions(-) rename Base/etc/{sudoers => plsusers} (82%) diff --git a/Base/etc/sudoers b/Base/etc/plsusers similarity index 82% rename from Base/etc/sudoers rename to Base/etc/plsusers index a301b3aac19..24fbad16118 100644 --- a/Base/etc/sudoers +++ b/Base/etc/plsusers @@ -1,4 +1,4 @@ -# sudoers file +# plsusers file # Put any users you want to allow to run programs as root here root anon diff --git a/Base/usr/share/man/man8/pls.md b/Base/usr/share/man/man8/pls.md index 3bf540d3741..131261340cf 100644 --- a/Base/usr/share/man/man8/pls.md +++ b/Base/usr/share/man/man8/pls.md @@ -11,7 +11,7 @@ $ pls [command] ## Description Executes a command as the root user (uid and gid 0), given that the user executing `pls` is located in -the sudoers file. +the plsusers file. It is possible to execute commands that contain hyphenated options via the use of `--`, which signifies the end of command options. For example: @@ -21,7 +21,7 @@ $ pls -- ls -la ``` ## Files -/etc/sudoers - List of users that can run `pls` +/etc/plsusers - List of users that can run `pls` ## Examples diff --git a/Documentation/BuildInstructions.md b/Documentation/BuildInstructions.md index 5ed034c2409..62879aaab6b 100644 --- a/Documentation/BuildInstructions.md +++ b/Documentation/BuildInstructions.md @@ -230,8 +230,8 @@ $ ninja run Note that the `anon` user is able to become `root` without password by default, as a development convenience. To prevent this, remove `anon` from the `wheel` group and he will no longer be able to run `/bin/su`. -`anon` is also, by default, located in `/etc/sudoers`, meaning that they will be able to execute with root permission using `pls`. -To prevent this, remove them from `/etc/sudoers`. +`anon` is also, by default, located in `/etc/plsusers`, meaning that they will be able to execute with root permission using `pls`. +To prevent this, remove them from `/etc/plsusers`. On Linux, QEMU is significantly faster if it's able to use KVM. The run script will automatically enable KVM if `/dev/kvm` exists and is readable+writable by the current user. diff --git a/Meta/build-root-filesystem.sh b/Meta/build-root-filesystem.sh index 9249eef1832..e80987b6383 100755 --- a/Meta/build-root-filesystem.sh +++ b/Meta/build-root-filesystem.sh @@ -50,6 +50,9 @@ chmod 660 mnt/etc/WindowServer.ini chown $window_uid:$window_gid mnt/etc/WindowServer.ini echo "/bin/sh" > mnt/etc/shells +chmod 0400 mnt/etc/plsusers +chown 0:0 mnt/etc/plsusers + chown 0:$wheel_gid mnt/bin/su chown 0:$wheel_gid mnt/bin/passwd chown 0:$wheel_gid mnt/bin/ping @@ -57,6 +60,7 @@ chown 0:$wheel_gid mnt/bin/traceroute chown 0:$phys_gid mnt/bin/keymap chown 0:$phys_gid mnt/bin/shutdown chown 0:$phys_gid mnt/bin/reboot +chown 0:$wheel_gid mnt/bin/pls chown 0:0 mnt/boot/Kernel chown 0:0 mnt/res/kernel.map chmod 0400 mnt/res/kernel.map diff --git a/Userland/Utilities/pls.cpp b/Userland/Utilities/pls.cpp index eb56f2d3337..7cfb1ecedbd 100644 --- a/Userland/Utilities/pls.cpp +++ b/Userland/Utilities/pls.cpp @@ -1,35 +1,17 @@ /* - * Copyright (c) 2021 Jesse Buhagiar - * All rights reserved. + * Copyright (c) 2021, Jesse Buhagiar * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR - * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER - * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, - * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include #include #include #include +#include #include #include #include @@ -37,38 +19,41 @@ #include #include +static constexpr mode_t EXPECTED_PERMS = (S_IFREG | S_IRUSR); + // Function Definitions extern "C" int main(int arch, char** argv); -int unveil_paths(const char*); +bool unveil_paths(const char*); // Unveil paths, given the current user's path and the command they want to execute -int unveil_paths(const char* command) +bool unveil_paths(const char* command) { - // Get the system path, split it and attempt to unveil all the paths. - // We do NOT error out on an invalid path - auto paths = String(getenv("PATH")).split(':'); - int num_unveils = 0; - char path_buf[256]; + bool did_unveil_ok = false; - // Unveil each path - for (const auto& path : paths) { - if (unveil(path.characters(), "x") == 0) - num_unveils++; - } + // Attempt to unveil command via `realpath` + auto* command_path = realpath(command, nullptr); - // Now unveil the command - auto command_path = realpath(command, &path_buf[0]); + // Command found via `realpath` (meaning it was probably a locally executed program) if (command_path) { if (unveil(command_path, "x") == 0) - num_unveils++; + did_unveil_ok = true; + + free(command_path); + return did_unveil_ok; } - return num_unveils; + // Okay, so we couldn't find the actual file specified by the user, let's + // instead search PATH for it... + auto command_path_system = Core::find_executable_in_path(command); + if (command_path_system.is_empty()) + return false; + + if (unveil(command_path_system.characters(), "x") == 0) + did_unveil_ok = true; + + return did_unveil_ok; } -// linusg: quaker: "please" feels quite long, how about "pls" :P -// "pls rm -r crap" has a nice ring to it -// lol int main(int argc, char** argv) { Vector command; @@ -82,12 +67,7 @@ int main(int argc, char** argv) return 1; } - if (seteuid(0) < 0) { - perror("seteuid"); - return 1; - } - - if (unveil("/etc/sudoers", "r") < 0) { + if (unveil("/etc/plsusers", "r") < 0) { perror("unveil"); return 1; } @@ -110,46 +90,66 @@ int main(int argc, char** argv) // Unveil all paths in the user's PATH, as well as the command they've specified. auto unveil_count = unveil_paths(command.at(0)); if (unveil_count == 0) { - warnln("Failed to unveil paths!"); + warnln("Error: Failed to unveil paths!"); return 1; } // Lock veil unveil(nullptr, nullptr); - const char* username = getlogin(); - auto sudoers_file_or_error = Core::File::open("/etc/sudoers", Core::IODevice::ReadOnly); - bool user_found = false; - if (sudoers_file_or_error.is_error()) { - warnln("couldn't open /etc/sudoers!"); + // Call `seteuid` so we can access `/etc/plsusers` + if (seteuid(0) < 0) { + perror("seteuid"); return 1; } - for (auto line = sudoers_file_or_error.value()->line_begin(); !line.at_end(); ++line) { + // Check the permissions and owner of /etc/plsusers. This ensures the integrity of the file. + struct stat pls_users_stat; + if (stat("/etc/plsusers", &pls_users_stat) < 0) { + perror("stat"); + return 1; + } + + if (pls_users_stat.st_mode != EXPECTED_PERMS) { + warnln("Error: /etc/plsusers has incorrect permissions."); + return 4; + } + + if (pls_users_stat.st_uid != 0 && pls_users_stat.st_gid != 0) { + warnln("Error: /etc/plsusers is not owned by root."); + return 4; + } + + auto pls_users_file_or_error = Core::File::open("/etc/plsusers", Core::OpenMode::ReadOnly); + if (pls_users_file_or_error.is_error()) { + warnln("Error: Could not open /etc/plsusers: {}", pls_users_file_or_error.error()); + return 1; + } + + const char* username = getlogin(); + bool user_found = false; + for (auto line = pls_users_file_or_error.value()->line_begin(); !line.at_end(); ++line) { auto line_str = *line; // Skip any comments if (line_str.starts_with("#")) continue; - // Our user is in the sudoers file! + // Our user is in the plsusers file! if (line_str.to_string() == username) { user_found = true; break; } } - // User isn't in the sudoer's file + // User isn't in the plsusers file if (!user_found) { - warnln("{} is not in the sudoers file!", username); + warnln("{} is not in the plsusers file!", username); return 2; } - // The user was in the sudoers file, now let's ask for their password to ensure that it's actually them... - uid_t current_uid = getuid(); - auto account_or_error = (username) - ? Core::Account::from_name(username) - : Core::Account::from_uid(current_uid); + // The user was in the plsusers file, now let's ask for their password to ensure that it's actually them... + auto account_or_error = Core::Account::from_name(username); if (account_or_error.is_error()) { warnln("Core::Account::from_name: {}", account_or_error.error()); @@ -157,6 +157,7 @@ int main(int argc, char** argv) } const auto& account = account_or_error.value(); + uid_t current_uid = getuid(); if (current_uid != 0 && account.has_password()) { auto password = Core::get_password(); if (password.is_error()) { @@ -171,13 +172,20 @@ int main(int argc, char** argv) } // TODO: Support swapping users instead of just defaulting to root - setgid(0); - setuid(0); + if (setgid(0) < 0) { + perror("setgid"); + return 1; + } + + if (setuid(0) < 0) { + perror("setuid"); + return 1; + } // Build the arguments list passed to `execvpe` Vector exec_args; - for (size_t i = 0; i < command.size(); i++) { - exec_args.append(command.at(i)); + for (const auto& arg : command) { + exec_args.append(arg); } // Always terminate with a NULL (to signal end of args list) @@ -185,26 +193,27 @@ int main(int argc, char** argv) // Build the environment arguments StringBuilder builder; + Vector env_args_str; - // Build SUDO_USER envvar - builder.append("SUDO_USER="); - builder.append(username); - auto sudo_user = builder.build(); - builder.clear(); + // TERM envvar + char* env_term = getenv("TERM"); - // Build SUDO_COMMAND envvar - builder.append("SUDO_COMMAND="); - builder.append(command.at(0)); - auto sudo_command = builder.build(); - builder.clear(); + if (env_term != nullptr) { + builder.append("TERM="); + builder.append(env_term); + env_args_str.append(builder.build()); + } - const char* envs[] = { "PROMPT=\\X\\u@\\h:\\w\\a\\e[33;1m\\h\\e[0m \\e[34;1m\\w\\e[0m \\p ", - "TERM=xterm", "PAGER=more", "PATH=/bin:/usr/bin:/usr/local/bin", - sudo_user.characters(), sudo_command.characters(), nullptr }; + Vector env_args; + for (auto& arg : env_args_str) { + env_args.append(arg.characters()); + } + + // Arguments list must be terminated with NULL argument + env_args.append(nullptr); // Execute the desired command - int rc = execvpe(command.at(0), const_cast(exec_args.data()), const_cast(envs)); - if (rc < 0) { + if (execvpe(command.at(0), const_cast(exec_args.data()), const_cast(env_args.data())) < 0) { perror("execvpe"); exit(1); }