From 1d4f85735cfa5c58121696eaf4790205d515c61f Mon Sep 17 00:00:00 2001 From: Alex Muscar Date: Thu, 19 Mar 2020 08:57:34 +0000 Subject: [PATCH] Kernel: Resolve relative paths when there is a veil (#1474) --- Kernel/FileSystem/VirtualFileSystem.cpp | 14 ++++++++++++-- Kernel/FileSystem/VirtualFileSystem.h | 1 + Kernel/Process.cpp | 12 ++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 65f28051da1f0a..ebf3a69100b85e 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -768,10 +768,20 @@ KResult VFS::validate_path_against_process_veil(StringView path, int options) KResultOr> VFS::resolve_path(StringView path, Custody& base, RefPtr* out_parent, int options, int symlink_recursion_level) { - auto result = validate_path_against_process_veil(path, options); + auto custody_or_error = resolve_path_without_veil(path, base, out_parent, options, symlink_recursion_level); + if (custody_or_error.is_error()) + return custody_or_error.error(); + + auto& custody = custody_or_error.value(); + auto result = validate_path_against_process_veil(custody->absolute_path(), options); if (result.is_error()) return result; + return custody; +} + +KResultOr> VFS::resolve_path_without_veil(StringView path, Custody& base, RefPtr* out_parent, int options, int symlink_recursion_level) +{ if (symlink_recursion_level >= symlink_recursion_limit) return KResult(-ELOOP); @@ -845,7 +855,7 @@ KResultOr> VFS::resolve_path(StringView path, Custody& ba remaining_path.append('.'); remaining_path.append(path.substring_view_starting_after_substring(part)); - return resolve_path(remaining_path.to_string(), *symlink_target.value(), out_parent, options, symlink_recursion_level + 1); + return resolve_path_without_veil(remaining_path.to_string(), *symlink_target.value(), out_parent, options, symlink_recursion_level + 1); } } diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h index 7aad464103ab72..ff390a2df0bf98 100644 --- a/Kernel/FileSystem/VirtualFileSystem.h +++ b/Kernel/FileSystem/VirtualFileSystem.h @@ -137,6 +137,7 @@ class VFS { Custody& root_custody(); KResultOr> resolve_path(StringView path, Custody& base, RefPtr* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0); + KResultOr> resolve_path_without_veil(StringView path, Custody& base, RefPtr* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0); private: friend class FileDescription; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 80932708a187b7..9219a955e379c7 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -4772,6 +4772,14 @@ int Process::sys$unveil(const Syscall::SC_unveil_params* user_params) if (path.value().is_empty() || path.value().characters()[0] != '/') return -EINVAL; + auto custody_or_error = VFS::the().resolve_path_without_veil(path.value(), root_directory()); + if (custody_or_error.is_error()) + // FIXME Should this be EINVAL? + return custody_or_error.error(); + + auto& custody = custody_or_error.value(); + auto new_unveiled_path = custody->absolute_path(); + auto permissions = validate_and_copy_string_from_user(params.permissions); if (permissions.is_null()) return -EFAULT; @@ -4798,7 +4806,7 @@ int Process::sys$unveil(const Syscall::SC_unveil_params* user_params) for (size_t i = 0; i < m_unveiled_paths.size(); ++i) { auto& unveiled_path = m_unveiled_paths[i]; - if (unveiled_path.path == path.value()) { + if (unveiled_path.path == new_unveiled_path) { if (new_permissions & ~unveiled_path.permissions) return -EPERM; unveiled_path.permissions = new_permissions; @@ -4806,7 +4814,7 @@ int Process::sys$unveil(const Syscall::SC_unveil_params* user_params) } } - m_unveiled_paths.append({ path.value(), new_permissions }); + m_unveiled_paths.append({ new_unveiled_path, new_permissions }); ASSERT(m_veil_state != VeilState::Locked); m_veil_state = VeilState::Dropped; return 0;