# HG changeset patch # User Bob Owen # Date 1631294898 -3600 # Fri Sep 10 18:28:18 2021 +0100 # Node ID adbc9b3051ab7f3c9360f65fe0fc26bd9d9dd499 # Parent 004b5bea4e78db7ecd665173ce4cf6aa0a1af199 Bug 1695556 p1: Allow reparse points in chromium sandbox code. Differential Revision: https://phabricator.services.mozilla.com/D135692 diff --git a/sandbox/win/src/filesystem_dispatcher.cc b/sandbox/win/src/filesystem_dispatcher.cc --- a/sandbox/win/src/filesystem_dispatcher.cc +++ b/sandbox/win/src/filesystem_dispatcher.cc @@ -92,17 +92,16 @@ bool FilesystemDispatcher::NtCreateFile( uint32_t create_options) { if ((create_options & FILE_VALID_OPTION_FLAGS) != create_options) { // Do not support brokering calls with special information in // NtCreateFile()'s ea_buffer (FILE_CONTAINS_EXTENDED_CREATE_INFORMATION). ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return false; } if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } EvalResult result = EvalPolicy(IpcTag::NTCREATEFILE, *name, desired_access, create_disposition == FILE_OPEN); HANDLE handle; ULONG_PTR io_information = 0; @@ -123,17 +122,16 @@ bool FilesystemDispatcher::NtCreateFile( bool FilesystemDispatcher::NtOpenFile(IPCInfo* ipc, std::wstring* name, uint32_t attributes, uint32_t desired_access, uint32_t share_access, uint32_t open_options) { if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } EvalResult result = EvalPolicy(IpcTag::NTOPENFILE, *name, desired_access, true); HANDLE handle; ULONG_PTR io_information = 0; @@ -154,17 +152,16 @@ bool FilesystemDispatcher::NtOpenFile(IP bool FilesystemDispatcher::NtQueryAttributesFile(IPCInfo* ipc, std::wstring* name, uint32_t attributes, CountedBuffer* info) { if (sizeof(FILE_BASIC_INFORMATION) != info->Size()) return false; if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } EvalResult result = EvalPolicy(IpcTag::NTQUERYATTRIBUTESFILE, *name); FILE_BASIC_INFORMATION* information = reinterpret_cast(info->Buffer()); @@ -184,17 +181,16 @@ bool FilesystemDispatcher::NtQueryAttrib bool FilesystemDispatcher::NtQueryFullAttributesFile(IPCInfo* ipc, std::wstring* name, uint32_t attributes, CountedBuffer* info) { if (sizeof(FILE_NETWORK_OPEN_INFORMATION) != info->Size()) return false; if (!PreProcessName(name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } EvalResult result = EvalPolicy(IpcTag::NTQUERYFULLATTRIBUTESFILE, *name); FILE_NETWORK_OPEN_INFORMATION* information = reinterpret_cast(info->Buffer()); static_cast(info_class); *nt_status = GetNtExports()->SetInformationFile( local_handle, io_block, file_info, length, file_info_class); return true; } bool PreProcessName(std::wstring* path) { @@ -227,17 +223,16 @@ bool FilesystemDispatcher::NtSetInformat if (!IsSupportedRenameCall(rename_info, length, info_class)) return false; std::wstring name; name.assign(rename_info->FileName, rename_info->FileNameLength / sizeof(rename_info->FileName[0])); if (!PreProcessName(&name)) { - // The path requested might contain a reparse point. ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } EvalResult result = EvalPolicy(IpcTag::NTSETINFO_RENAME, name); IO_STATUS_BLOCK* io_status = reinterpret_cast(status->Buffer()); diff --git a/sandbox/win/src/filesystem_policy.cc b/sandbox/win/src/filesystem_policy.cc --- a/sandbox/win/src/filesystem_policy.cc +++ b/sandbox/win/src/filesystem_policy.cc @@ -5,16 +5,17 @@ #include "sandbox/win/src/filesystem_policy.h" #include #include #include #include +#include #include #include "base/notreached.h" #include "base/win/scoped_handle.h" #include "sandbox/win/src/internal_types.h" #include "sandbox/win/src/ipc_tags.h" #include "sandbox/win/src/nt_internals.h" #include "sandbox/win/src/policy_engine_opcodes.h" @@ -59,22 +60,16 @@ NTSTATUS NtCreateFileInTarget(HANDLE* ta NTSTATUS status = GetNtExports()->CreateFile( &local_handle, desired_access, obj_attributes, io_status_block, nullptr, file_attributes, share_access, create_disposition, create_options, ea_buffer, ea_length); if (!NT_SUCCESS(status)) { return status; } - if (!SameObject(local_handle, obj_attributes->ObjectName->Buffer)) { - // The handle points somewhere else. Fail the operation. - ::CloseHandle(local_handle); - return STATUS_ACCESS_DENIED; - } - if (!::DuplicateHandle(::GetCurrentProcess(), local_handle, target_process, target_file_handle, 0, false, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { return STATUS_ACCESS_DENIED; } return STATUS_SUCCESS; } @@ -285,23 +280,32 @@ bool FileSystemPolicy::SetInformationFil static_cast(info_class); *nt_status = GetNtExports()->SetInformationFile( local_handle, io_block, file_info, length, file_info_class); return true; } bool PreProcessName(std::wstring* path) { - ConvertToLongPath(path); + // We now allow symbolic links to be opened via the broker, so we can no + // longer rely on the same object check where we checked the path of the + // opened file against the original. We don't specify a root when creating + // OBJECT_ATTRIBUTES from file names for brokering so they must be fully + // qualified and we can just check for the parent directory double dot between + // two backslashes. NtCreateFile doesn't seem to allow it anyway, but this is + // just an extra precaution. It also doesn't seem to allow the forward slash, + // but this is also used for checking policy rules, so we just replace forward + // slashes with backslashes. + std::replace(path->begin(), path->end(), L'/', L'\\'); + if (path->find(L"\\..\\") != std::wstring::npos) { + return false; + } - if (ERROR_NOT_A_REPARSE_POINT == IsReparsePoint(*path)) - return true; - - // We can't process a reparsed file. - return false; + ConvertToLongPath(path); + return true; } std::wstring FixNTPrefixForMatch(const std::wstring& name) { std::wstring mod_name = name; // NT prefix escaped for rule matcher const wchar_t kNTPrefixEscaped[] = L"\\/?/?\\"; const int kNTPrefixEscapedLen = base::size(kNTPrefixEscaped) - 1;