27_allow_reparse_points.patch (7573B)
1 # HG changeset patch 2 # User Bob Owen <bobowencode@gmail.com> 3 # Date 1631294898 -3600 4 # Fri Sep 10 18:28:18 2021 +0100 5 # Node ID adbc9b3051ab7f3c9360f65fe0fc26bd9d9dd499 6 # Parent 004b5bea4e78db7ecd665173ce4cf6aa0a1af199 7 Bug 1695556 p1: Allow reparse points in chromium sandbox code. 8 9 Differential Revision: https://phabricator.services.mozilla.com/D135692 10 11 diff --git a/sandbox/win/src/filesystem_dispatcher.cc b/sandbox/win/src/filesystem_dispatcher.cc 12 --- a/sandbox/win/src/filesystem_dispatcher.cc 13 +++ b/sandbox/win/src/filesystem_dispatcher.cc 14 @@ -92,17 +92,16 @@ bool FilesystemDispatcher::NtCreateFile( 15 uint32_t create_options) { 16 if ((create_options & FILE_VALID_OPTION_FLAGS) != create_options) { 17 // Do not support brokering calls with special information in 18 // NtCreateFile()'s ea_buffer (FILE_CONTAINS_EXTENDED_CREATE_INFORMATION). 19 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 20 return false; 21 } 22 if (!PreProcessName(name)) { 23 - // The path requested might contain a reparse point. 24 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 25 return true; 26 } 27 28 EvalResult result = EvalPolicy(IpcTag::NTCREATEFILE, *name, desired_access, 29 create_disposition == FILE_OPEN); 30 HANDLE handle; 31 ULONG_PTR io_information = 0; 32 @@ -123,17 +122,16 @@ bool FilesystemDispatcher::NtCreateFile( 33 34 bool FilesystemDispatcher::NtOpenFile(IPCInfo* ipc, 35 std::wstring* name, 36 uint32_t attributes, 37 uint32_t desired_access, 38 uint32_t share_access, 39 uint32_t open_options) { 40 if (!PreProcessName(name)) { 41 - // The path requested might contain a reparse point. 42 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 43 return true; 44 } 45 46 EvalResult result = 47 EvalPolicy(IpcTag::NTOPENFILE, *name, desired_access, true); 48 HANDLE handle; 49 ULONG_PTR io_information = 0; 50 @@ -154,17 +152,16 @@ bool FilesystemDispatcher::NtOpenFile(IP 51 bool FilesystemDispatcher::NtQueryAttributesFile(IPCInfo* ipc, 52 std::wstring* name, 53 uint32_t attributes, 54 CountedBuffer* info) { 55 if (sizeof(FILE_BASIC_INFORMATION) != info->Size()) 56 return false; 57 58 if (!PreProcessName(name)) { 59 - // The path requested might contain a reparse point. 60 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 61 return true; 62 } 63 64 EvalResult result = EvalPolicy(IpcTag::NTQUERYATTRIBUTESFILE, *name); 65 66 FILE_BASIC_INFORMATION* information = 67 reinterpret_cast<FILE_BASIC_INFORMATION*>(info->Buffer()); 68 @@ -184,17 +181,16 @@ bool FilesystemDispatcher::NtQueryAttrib 69 bool FilesystemDispatcher::NtQueryFullAttributesFile(IPCInfo* ipc, 70 std::wstring* name, 71 uint32_t attributes, 72 CountedBuffer* info) { 73 if (sizeof(FILE_NETWORK_OPEN_INFORMATION) != info->Size()) 74 return false; 75 76 if (!PreProcessName(name)) { 77 - // The path requested might contain a reparse point. 78 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 79 return true; 80 } 81 82 EvalResult result = EvalPolicy(IpcTag::NTQUERYFULLATTRIBUTESFILE, *name); 83 84 FILE_NETWORK_OPEN_INFORMATION* information = 85 reinterpret_cast<FILE_NETWORK_OPEN_INFORMATION*>(info->Buffer()); 86 static_cast<FILE_INFORMATION_CLASS>(info_class); 87 *nt_status = GetNtExports()->SetInformationFile( 88 local_handle, io_block, file_info, length, file_info_class); 89 90 return true; 91 } 92 93 bool PreProcessName(std::wstring* path) { 94 @@ -227,17 +223,16 @@ bool FilesystemDispatcher::NtSetInformat 95 96 if (!IsSupportedRenameCall(rename_info, length, info_class)) 97 return false; 98 99 std::wstring name; 100 name.assign(rename_info->FileName, 101 rename_info->FileNameLength / sizeof(rename_info->FileName[0])); 102 if (!PreProcessName(&name)) { 103 - // The path requested might contain a reparse point. 104 ipc->return_info.nt_status = STATUS_ACCESS_DENIED; 105 return true; 106 } 107 108 EvalResult result = EvalPolicy(IpcTag::NTSETINFO_RENAME, name); 109 110 IO_STATUS_BLOCK* io_status = 111 reinterpret_cast<IO_STATUS_BLOCK*>(status->Buffer()); 112 diff --git a/sandbox/win/src/filesystem_policy.cc b/sandbox/win/src/filesystem_policy.cc 113 --- a/sandbox/win/src/filesystem_policy.cc 114 +++ b/sandbox/win/src/filesystem_policy.cc 115 @@ -5,16 +5,17 @@ 116 #include "sandbox/win/src/filesystem_policy.h" 117 118 #include <windows.h> 119 #include <winternl.h> 120 121 #include <ntstatus.h> 122 #include <stdint.h> 123 124 +#include <algorithm> 125 #include <string> 126 127 #include "base/notreached.h" 128 #include "base/win/scoped_handle.h" 129 #include "sandbox/win/src/internal_types.h" 130 #include "sandbox/win/src/ipc_tags.h" 131 #include "sandbox/win/src/nt_internals.h" 132 #include "sandbox/win/src/policy_engine_opcodes.h" 133 @@ -59,22 +60,16 @@ NTSTATUS NtCreateFileInTarget(HANDLE* ta 134 NTSTATUS status = GetNtExports()->CreateFile( 135 &local_handle, desired_access, obj_attributes, io_status_block, nullptr, 136 file_attributes, share_access, create_disposition, create_options, 137 ea_buffer, ea_length); 138 if (!NT_SUCCESS(status)) { 139 return status; 140 } 141 142 - if (!SameObject(local_handle, obj_attributes->ObjectName->Buffer)) { 143 - // The handle points somewhere else. Fail the operation. 144 - ::CloseHandle(local_handle); 145 - return STATUS_ACCESS_DENIED; 146 - } 147 - 148 if (!::DuplicateHandle(::GetCurrentProcess(), local_handle, target_process, 149 target_file_handle, 0, false, 150 DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { 151 return STATUS_ACCESS_DENIED; 152 } 153 return STATUS_SUCCESS; 154 } 155 156 @@ -285,23 +280,32 @@ bool FileSystemPolicy::SetInformationFil 157 static_cast<FILE_INFORMATION_CLASS>(info_class); 158 *nt_status = GetNtExports()->SetInformationFile( 159 local_handle, io_block, file_info, length, file_info_class); 160 161 return true; 162 } 163 164 bool PreProcessName(std::wstring* path) { 165 - ConvertToLongPath(path); 166 + // We now allow symbolic links to be opened via the broker, so we can no 167 + // longer rely on the same object check where we checked the path of the 168 + // opened file against the original. We don't specify a root when creating 169 + // OBJECT_ATTRIBUTES from file names for brokering so they must be fully 170 + // qualified and we can just check for the parent directory double dot between 171 + // two backslashes. NtCreateFile doesn't seem to allow it anyway, but this is 172 + // just an extra precaution. It also doesn't seem to allow the forward slash, 173 + // but this is also used for checking policy rules, so we just replace forward 174 + // slashes with backslashes. 175 + std::replace(path->begin(), path->end(), L'/', L'\\'); 176 + if (path->find(L"\\..\\") != std::wstring::npos) { 177 + return false; 178 + } 179 180 - if (ERROR_NOT_A_REPARSE_POINT == IsReparsePoint(*path)) 181 - return true; 182 - 183 - // We can't process a reparsed file. 184 - return false; 185 + ConvertToLongPath(path); 186 + return true; 187 } 188 189 std::wstring FixNTPrefixForMatch(const std::wstring& name) { 190 std::wstring mod_name = name; 191 192 // NT prefix escaped for rule matcher 193 const wchar_t kNTPrefixEscaped[] = L"\\/?/?\\"; 194 const int kNTPrefixEscapedLen = base::size(kNTPrefixEscaped) - 1;