Browse Source

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.
Liav A 2 years ago
parent
commit
b40b1c8d93
2 changed files with 3 additions and 3 deletions
  1. 2 2
      Kernel/FileSystem/VirtualFileSystem.cpp
  2. 1 1
      Userland/Utilities/userdel.cpp

+ 2 - 2
Kernel/FileSystem/VirtualFileSystem.cpp

@@ -806,7 +806,7 @@ ErrorOr<void> VirtualFileSystem::link(Credentials const& credentials, StringView
 ErrorOr<void> VirtualFileSystem::unlink(Credentials const& credentials, StringView path, Custody& base)
 {
     RefPtr<Custody> 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<void> VirtualFileSystem::symlink(Credentials const& credentials, StringV
 ErrorOr<void> VirtualFileSystem::rmdir(Credentials const& credentials, StringView path, Custody& base)
 {
     RefPtr<Custody> 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);

+ 1 - 1
Userland/Utilities/userdel.cpp

@@ -35,7 +35,7 @@ ErrorOr<int> 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));