From b40b1c8d937af9fc257244853a682952aa3d675f Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 2 Jun 2023 02:32:31 +0300 Subject: [PATCH] Kernel+Userland: Ensure proper unveil permissions before using rm/rmdir When deleting a directory, the rmdir syscall should fail if the path was unveiled without the 'c' permission. This matches the same behavior that OpenBSD enforces when doing this kind of operation. When deleting a file, the unlink syscall should fail if the path was unveiled without the 'w' permission, to ensure that userspace is aware of the possibility of removing a file only when the path was unveiled as writable. When using the userdel utility, we now unveil that directory path with the unveil 'c' permission so removal of an account home directory is done properly. --- Kernel/FileSystem/VirtualFileSystem.cpp | 4 ++-- Userland/Utilities/userdel.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 006e23fc88..d4c81846f3 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -806,7 +806,7 @@ ErrorOr VirtualFileSystem::link(Credentials const& credentials, StringView ErrorOr VirtualFileSystem::unlink(Credentials const& credentials, StringView path, Custody& base) { RefPtr parent_custody; - auto custody = TRY(resolve_path(credentials, path, base, &parent_custody, O_NOFOLLOW_NOERROR | O_UNLINK_INTERNAL)); + auto custody = TRY(resolve_path(credentials, path, base, &parent_custody, O_WRONLY | O_NOFOLLOW_NOERROR | O_UNLINK_INTERNAL)); auto& inode = custody->inode(); if (inode.is_directory()) @@ -875,7 +875,7 @@ ErrorOr VirtualFileSystem::symlink(Credentials const& credentials, StringV ErrorOr VirtualFileSystem::rmdir(Credentials const& credentials, StringView path, Custody& base) { RefPtr parent_custody; - auto custody = TRY(resolve_path(credentials, path, base, &parent_custody)); + auto custody = TRY(resolve_path(credentials, path, base, &parent_custody, O_CREAT)); auto& inode = custody->inode(); auto last_component = KLexicalPath::basename(path); diff --git a/Userland/Utilities/userdel.cpp b/Userland/Utilities/userdel.cpp index 472b90d292..f8d29b027f 100644 --- a/Userland/Utilities/userdel.cpp +++ b/Userland/Utilities/userdel.cpp @@ -35,7 +35,7 @@ ErrorOr serenity_main(Main::Arguments arguments) auto& target_account = account_or_error.value(); if (remove_home) - TRY(Core::System::unveil(target_account.home_directory(), "r"sv)); + TRY(Core::System::unveil(target_account.home_directory(), "c"sv)); TRY(Core::System::unveil(nullptr, nullptr));