From e77485c628a192ce9818c59b6ec362d364d25ce1 Mon Sep 17 00:00:00 2001 From: Toshihito Kikuchi Date: Fri, 3 Jul 2020 19:15:53 +0000 Subject: [PATCH] Bug 1639030 - Part 3: Roll-up patch to apply remaining mozilla changes to chromium sandbox. r=bobowen This commit applies patches under security/sandbox/chromium-shim/patches/after_update/. Differential Revision: https://phabricator.services.mozilla.com/D79561 --- .../base/win/scoped_handle_verifier.cc | 6 + .../sandbox/win/src/filesystem_dispatcher.cc | 50 ++++++++ .../win/src/filesystem_interception.cc | 48 ++++--- .../sandbox/win/src/handle_interception.cc | 3 + .../win/src/named_pipe_interception.cc | 4 + .../win/src/process_thread_interception.cc | 13 ++ .../sandbox/win/src/registry_interception.cc | 25 ++++ .../sandbox/win/src/registry_policy.cc | 3 +- .../sandbox/win/src/signed_interception.cc | 4 + .../sandbox/win/src/sync_interception.cc | 15 +++ .../sandbox/win/src/target_process.cc | 35 ++--- .../chromium/sandbox/win/src/win_utils.cc | 120 +++++++++++------- .../chromium/sandbox/win/src/win_utils.h | 3 +- 13 files changed, 252 insertions(+), 77 deletions(-) diff --git a/security/sandbox/chromium/base/win/scoped_handle_verifier.cc b/security/sandbox/chromium/base/win/scoped_handle_verifier.cc index 21471e14a3bf..316606c0bfa4 100644 --- a/security/sandbox/chromium/base/win/scoped_handle_verifier.cc +++ b/security/sandbox/chromium/base/win/scoped_handle_verifier.cc @@ -71,7 +71,13 @@ ScopedHandleVerifier* ScopedHandleVerifier::Get() { bool CloseHandleWrapper(HANDLE handle) { if (!::CloseHandle(handle)) + // Making this DCHECK on non-Nighly as we are hitting this frequently, + // looks like we are closing handles twice somehow. See bug 1564899. +#if defined(NIGHTLY_BUILD) CHECK(false); // CloseHandle failed. +#else + DCHECK(false); // CloseHandle failed. +#endif return true; } diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc index ea1c3314ba27..ff0f8885cd64 100644 --- a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc +++ b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc @@ -17,6 +17,8 @@ #include "sandbox/win/src/sandbox.h" #include "sandbox/win/src/sandbox_nt_util.h" +#include "mozilla/sandboxing/permissionsService.h" + namespace sandbox { FilesystemDispatcher::FilesystemDispatcher(PolicyBase* policy_base) @@ -110,6 +112,16 @@ bool FilesystemDispatcher::NtCreateFile(IPCInfo* ipc, // knows what to do. EvalResult result = policy_base_->EvalPolicy(IpcTag::NTCREATEFILE, params.GetBase()); + + // If the policies forbid access (any result other than ASK_BROKER), + // then check for user-granted access to file. + if (ASK_BROKER != result && + mozilla::sandboxing::PermissionsService::GetInstance()-> + UserGrantedFileAccess(ipc->client_info->process_id, filename, + desired_access, create_disposition)) { + result = ASK_BROKER; + } + HANDLE handle; ULONG_PTR io_information = 0; NTSTATUS nt_status; @@ -155,6 +167,16 @@ bool FilesystemDispatcher::NtOpenFile(IPCInfo* ipc, // knows what to do. EvalResult result = policy_base_->EvalPolicy(IpcTag::NTOPENFILE, params.GetBase()); + + // If the policies forbid access (any result other than ASK_BROKER), + // then check for user-granted access to file. + if (ASK_BROKER != result && + mozilla::sandboxing::PermissionsService::GetInstance()->UserGrantedFileAccess( + ipc->client_info->process_id, filename, + desired_access, create_disposition)) { + result = ASK_BROKER; + } + HANDLE handle; ULONG_PTR io_information = 0; NTSTATUS nt_status; @@ -196,6 +218,15 @@ bool FilesystemDispatcher::NtQueryAttributesFile(IPCInfo* ipc, EvalResult result = policy_base_->EvalPolicy(IpcTag::NTQUERYATTRIBUTESFILE, params.GetBase()); + // If the policies forbid access (any result other than ASK_BROKER), + // then check for user-granted access to file. + if (ASK_BROKER != result && + mozilla::sandboxing::PermissionsService::GetInstance()-> + UserGrantedFileAccess(ipc->client_info->process_id, filename, + 0, 0)) { + result = ASK_BROKER; + } + FILE_BASIC_INFORMATION* information = reinterpret_cast(info->Buffer()); NTSTATUS nt_status; @@ -236,6 +267,15 @@ bool FilesystemDispatcher::NtQueryFullAttributesFile(IPCInfo* ipc, EvalResult result = policy_base_->EvalPolicy( IpcTag::NTQUERYFULLATTRIBUTESFILE, params.GetBase()); + // If the policies forbid access (any result other than ASK_BROKER), + // then check for user-granted access to file. + if (ASK_BROKER != result && + mozilla::sandboxing::PermissionsService::GetInstance()-> + UserGrantedFileAccess(ipc->client_info->process_id, filename, + 0, 0)) { + result = ASK_BROKER; + } + FILE_NETWORK_OPEN_INFORMATION* information = reinterpret_cast(info->Buffer()); NTSTATUS nt_status; @@ -289,6 +329,16 @@ bool FilesystemDispatcher::NtSetInformationFile(IPCInfo* ipc, EvalResult result = policy_base_->EvalPolicy(IpcTag::NTSETINFO_RENAME, params.GetBase()); + // If the policies forbid access (any result other than ASK_BROKER), + // then check for user-granted write access to file. We only permit + // the FileRenameInformation action. + if (ASK_BROKER != result && info_class == FileRenameInformation && + mozilla::sandboxing::PermissionsService::GetInstance()-> + UserGrantedFileAccess(ipc->client_info->process_id, filename, + FILE_WRITE_ATTRIBUTES, 0)) { + result = ASK_BROKER; + } + IO_STATUS_BLOCK* io_status = reinterpret_cast(status->Buffer()); NTSTATUS nt_status; diff --git a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc index 0f28b0c96dcd..1649eba6184a 100644 --- a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc @@ -15,6 +15,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" // This status occurs when trying to access a network share on the machine from // which it is shared. @@ -42,6 +43,10 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, STATUS_NETWORK_OPEN_RESTRICTION != status) return status; + mozilla::sandboxing::LogBlocked("NtCreateFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -75,9 +80,6 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, params[OpenFile::OPTIONS] = ParamPickerMake(options_uint32); params[OpenFile::BROKER] = ParamPickerMake(broker); - if (!QueryBroker(IpcTag::NTCREATEFILE, params.GetBase())) - break; - SharedMemIPCClient ipc(memory); CrossCallReturn answer = {0}; // The following call must match in the parameters with @@ -101,6 +103,9 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtCreateFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -120,6 +125,10 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, STATUS_NETWORK_OPEN_RESTRICTION != status) return status; + mozilla::sandboxing::LogBlocked("NtOpenFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -153,9 +162,6 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, params[OpenFile::OPTIONS] = ParamPickerMake(options_uint32); params[OpenFile::BROKER] = ParamPickerMake(broker); - if (!QueryBroker(IpcTag::NTOPENFILE, params.GetBase())) - break; - SharedMemIPCClient ipc(memory); CrossCallReturn answer = {0}; ResultCode code = @@ -176,6 +182,9 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtOpenFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -191,6 +200,10 @@ TargetNtQueryAttributesFile(NtQueryAttributesFileFunction orig_QueryAttributes, STATUS_NETWORK_OPEN_RESTRICTION != status) return status; + mozilla::sandboxing::LogBlocked("NtQueryAttributesFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -219,9 +232,6 @@ TargetNtQueryAttributesFile(NtQueryAttributesFileFunction orig_QueryAttributes, params[FileName::NAME] = ParamPickerMake(name_ptr); params[FileName::BROKER] = ParamPickerMake(broker); - if (!QueryBroker(IpcTag::NTQUERYATTRIBUTESFILE, params.GetBase())) - break; - SharedMemIPCClient ipc(memory); CrossCallReturn answer = {0}; ResultCode code = CrossCall(ipc, IpcTag::NTQUERYATTRIBUTESFILE, name.get(), @@ -232,6 +242,9 @@ TargetNtQueryAttributesFile(NtQueryAttributesFileFunction orig_QueryAttributes, status = answer.nt_status; + mozilla::sandboxing::LogAllowed("NtQueryAttributesFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -248,6 +261,10 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( STATUS_NETWORK_OPEN_RESTRICTION != status) return status; + mozilla::sandboxing::LogBlocked("NtQueryFullAttributesFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -277,9 +294,6 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( params[FileName::NAME] = ParamPickerMake(name_ptr); params[FileName::BROKER] = ParamPickerMake(broker); - if (!QueryBroker(IpcTag::NTQUERYFULLATTRIBUTESFILE, params.GetBase())) - break; - SharedMemIPCClient ipc(memory); CrossCallReturn answer = {0}; ResultCode code = CrossCall(ipc, IpcTag::NTQUERYFULLATTRIBUTESFILE, @@ -289,6 +303,10 @@ NTSTATUS WINAPI TargetNtQueryFullAttributesFile( break; status = answer.nt_status; + + mozilla::sandboxing::LogAllowed("NtQueryFullAttributesFile", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -307,6 +325,8 @@ TargetNtSetInformationFile(NtSetInformationFileFunction orig_SetInformationFile, if (STATUS_ACCESS_DENIED != status) return status; + mozilla::sandboxing::LogBlocked("NtSetInformationFile"); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -353,9 +373,6 @@ TargetNtSetInformationFile(NtSetInformationFileFunction orig_SetInformationFile, params[FileName::NAME] = ParamPickerMake(name_ptr); params[FileName::BROKER] = ParamPickerMake(broker); - if (!QueryBroker(IpcTag::NTSETINFO_RENAME, params.GetBase())) - break; - InOutCountedBuffer io_status_buffer(io_status, sizeof(IO_STATUS_BLOCK)); // This is actually not an InOut buffer, only In, but using InOut facility // really helps to simplify the code. @@ -371,6 +388,7 @@ TargetNtSetInformationFile(NtSetInformationFileFunction orig_SetInformationFile, break; status = answer.nt_status; + mozilla::sandboxing::LogAllowed("NtSetInformationFile"); } while (false); return status; diff --git a/security/sandbox/chromium/sandbox/win/src/handle_interception.cc b/security/sandbox/chromium/sandbox/win/src/handle_interception.cc index ff9401ee876e..53db4a8b2761 100644 --- a/security/sandbox/chromium/sandbox/win/src/handle_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/handle_interception.cc @@ -10,6 +10,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -34,10 +35,12 @@ ResultCode DuplicateHandleProxy(HANDLE source_handle, if (answer.win32_result) { ::SetLastError(answer.win32_result); + mozilla::sandboxing::LogBlocked("DuplicateHandle"); return SBOX_ERROR_GENERIC; } *target_handle = answer.handle; + mozilla::sandboxing::LogAllowed("DuplicateHandle"); return SBOX_ALL_OK; } diff --git a/security/sandbox/chromium/sandbox/win/src/named_pipe_interception.cc b/security/sandbox/chromium/sandbox/win/src/named_pipe_interception.cc index c1ca07a3a52a..aa3d5dc35852 100644 --- a/security/sandbox/chromium/sandbox/win/src/named_pipe_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/named_pipe_interception.cc @@ -12,6 +12,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -31,6 +32,8 @@ TargetCreateNamedPipeW(CreateNamedPipeWFunction orig_CreateNamedPipeW, if (INVALID_HANDLE_VALUE != pipe) return pipe; + mozilla::sandboxing::LogBlocked("CreateNamedPipeW", pipe_name); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return INVALID_HANDLE_VALUE; @@ -66,6 +69,7 @@ TargetCreateNamedPipeW(CreateNamedPipeWFunction orig_CreateNamedPipeW, if (ERROR_SUCCESS != answer.win32_result) return INVALID_HANDLE_VALUE; + mozilla::sandboxing::LogAllowed("CreateNamedPipeW", pipe_name); return answer.handle; } while (false); diff --git a/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc b/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc index 29d0cada58c4..0bd3d5a8d21b 100644 --- a/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc @@ -15,6 +15,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -32,6 +33,7 @@ NTSTATUS WINAPI TargetNtOpenThread(NtOpenThreadFunction orig_OpenThread, if (NT_SUCCESS(status)) return status; + mozilla::sandboxing::LogBlocked("NtOpenThread"); do { if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) break; @@ -96,6 +98,7 @@ NTSTATUS WINAPI TargetNtOpenThread(NtOpenThreadFunction orig_OpenThread, break; } + mozilla::sandboxing::LogAllowed("NtOpenThread"); return answer.nt_status; } while (false); @@ -181,6 +184,7 @@ TargetNtOpenProcessToken(NtOpenProcessTokenFunction orig_OpenProcessToken, if (NT_SUCCESS(status)) return status; + mozilla::sandboxing::LogBlocked("NtOpenProcessToken"); do { if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) break; @@ -212,6 +216,7 @@ TargetNtOpenProcessToken(NtOpenProcessTokenFunction orig_OpenProcessToken, break; } + mozilla::sandboxing::LogAllowed("NtOpenProcessToken"); return answer.nt_status; } while (false); @@ -229,6 +234,7 @@ TargetNtOpenProcessTokenEx(NtOpenProcessTokenExFunction orig_OpenProcessTokenEx, if (NT_SUCCESS(status)) return status; + mozilla::sandboxing::LogBlocked("NtOpenProcessTokenEx"); do { if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) break; @@ -260,6 +266,7 @@ TargetNtOpenProcessTokenEx(NtOpenProcessTokenExFunction orig_OpenProcessTokenEx, break; } + mozilla::sandboxing::LogAllowed("NtOpenProcessTokenEx"); return answer.nt_status; } while (false); @@ -285,6 +292,8 @@ BOOL WINAPI TargetCreateProcessW(CreateProcessWFunction orig_CreateProcessW, return true; } + mozilla::sandboxing::LogBlocked("CreateProcessW", application_name); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return false; @@ -325,6 +334,7 @@ BOOL WINAPI TargetCreateProcessW(CreateProcessWFunction orig_CreateProcessW, if (ERROR_SUCCESS != answer.win32_result) return false; + mozilla::sandboxing::LogAllowed("CreateProcessW", application_name); return true; } while (false); @@ -351,6 +361,8 @@ BOOL WINAPI TargetCreateProcessA(CreateProcessAFunction orig_CreateProcessA, return true; } + mozilla::sandboxing::LogBlocked("CreateProcessA", application_name); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return false; @@ -425,6 +437,7 @@ BOOL WINAPI TargetCreateProcessA(CreateProcessAFunction orig_CreateProcessA, if (ERROR_SUCCESS != answer.win32_result) return false; + mozilla::sandboxing::LogAllowed("CreateProcessA", application_name); return true; } while (false); diff --git a/security/sandbox/chromium/sandbox/win/src/registry_interception.cc b/security/sandbox/chromium/sandbox/win/src/registry_interception.cc index ec09aeb79388..d8bb82949b39 100644 --- a/security/sandbox/chromium/sandbox/win/src/registry_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/registry_interception.cc @@ -14,6 +14,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -32,6 +33,12 @@ NTSTATUS WINAPI TargetNtCreateKey(NtCreateKeyFunction orig_CreateKey, if (NT_SUCCESS(status)) return status; + if (STATUS_OBJECT_NAME_NOT_FOUND != status) { + mozilla::sandboxing::LogBlocked("NtCreateKey", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + } + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -119,6 +126,9 @@ NTSTATUS WINAPI TargetNtCreateKey(NtCreateKeyFunction orig_CreateKey, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtCreateKey", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -198,6 +208,9 @@ NTSTATUS WINAPI CommonNtOpenKey(NTSTATUS status, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtOpenKey[Ex]", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -212,6 +225,12 @@ NTSTATUS WINAPI TargetNtOpenKey(NtOpenKeyFunction orig_OpenKey, if (NT_SUCCESS(status)) return status; + if (STATUS_OBJECT_NAME_NOT_FOUND != status) { + mozilla::sandboxing::LogBlocked("NtOpenKey", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + } + return CommonNtOpenKey(status, key, desired_access, object_attributes); } @@ -230,6 +249,12 @@ NTSTATUS WINAPI TargetNtOpenKeyEx(NtOpenKeyExFunction orig_OpenKeyEx, if (NT_SUCCESS(status) || open_options != 0) return status; + if (STATUS_OBJECT_NAME_NOT_FOUND != status) { + mozilla::sandboxing::LogBlocked("NtOpenKeyEx", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + } + return CommonNtOpenKey(status, key, desired_access, object_attributes); } diff --git a/security/sandbox/chromium/sandbox/win/src/registry_policy.cc b/security/sandbox/chromium/sandbox/win/src/registry_policy.cc index 1379501fb3a4..212ff713c44b 100644 --- a/security/sandbox/chromium/sandbox/win/src/registry_policy.cc +++ b/security/sandbox/chromium/sandbox/win/src/registry_policy.cc @@ -20,7 +20,8 @@ namespace { static const uint32_t kAllowedRegFlags = KEY_QUERY_VALUE | KEY_ENUMERATE_SUB_KEYS | KEY_NOTIFY | KEY_READ | - GENERIC_READ | GENERIC_EXECUTE | READ_CONTROL; + GENERIC_READ | GENERIC_EXECUTE | READ_CONTROL | KEY_WOW64_64KEY | + KEY_WOW64_32KEY; // Opens the key referenced by |obj_attributes| with |access| and // checks what permission was given. Remove the WRITE flags and update diff --git a/security/sandbox/chromium/sandbox/win/src/signed_interception.cc b/security/sandbox/chromium/sandbox/win/src/signed_interception.cc index 86c4b598aa36..4f4310d6052b 100644 --- a/security/sandbox/chromium/sandbox/win/src/signed_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/signed_interception.cc @@ -14,6 +14,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -42,6 +43,8 @@ TargetNtCreateSection(NtCreateSectionFunction orig_CreateSection, if (allocation_attributes != SEC_IMAGE) break; + mozilla::sandboxing::LogBlocked("NtCreateSection"); + // IPC must be fully started. void* memory = GetGlobalIPCMemory(); if (!memory) @@ -78,6 +81,7 @@ TargetNtCreateSection(NtCreateSectionFunction orig_CreateSection, __try { *section_handle = answer.handle; + mozilla::sandboxing::LogAllowed("NtCreateSection"); return answer.nt_status; } __except (EXCEPTION_EXECUTE_HANDLER) { break; diff --git a/security/sandbox/chromium/sandbox/win/src/sync_interception.cc b/security/sandbox/chromium/sandbox/win/src/sync_interception.cc index d062ecb7b35b..9ec680e2fd1e 100644 --- a/security/sandbox/chromium/sandbox/win/src/sync_interception.cc +++ b/security/sandbox/chromium/sandbox/win/src/sync_interception.cc @@ -14,6 +14,7 @@ #include "sandbox/win/src/sandbox_nt_util.h" #include "sandbox/win/src/sharedmem_ipc_client.h" #include "sandbox/win/src/target_services.h" +#include "mozilla/sandboxing/sandboxLogging.h" namespace sandbox { @@ -64,6 +65,10 @@ NTSTATUS WINAPI TargetNtCreateEvent(NtCreateEventFunction orig_CreateEvent, if (status != STATUS_ACCESS_DENIED || !object_attributes) return status; + mozilla::sandboxing::LogBlocked("NtCreatEvent", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -102,6 +107,9 @@ NTSTATUS WINAPI TargetNtCreateEvent(NtCreateEventFunction orig_CreateEvent, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtCreateEvent", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; @@ -116,6 +124,10 @@ NTSTATUS WINAPI TargetNtOpenEvent(NtOpenEventFunction orig_OpenEvent, if (status != STATUS_ACCESS_DENIED || !object_attributes) return status; + mozilla::sandboxing::LogBlocked("NtOpenEvent", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); + // We don't trust that the IPC can work this early. if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled()) return status; @@ -154,6 +166,9 @@ NTSTATUS WINAPI TargetNtOpenEvent(NtOpenEventFunction orig_OpenEvent, } __except (EXCEPTION_EXECUTE_HANDLER) { break; } + mozilla::sandboxing::LogAllowed("NtOpenEvent", + object_attributes->ObjectName->Buffer, + object_attributes->ObjectName->Length); } while (false); return status; diff --git a/security/sandbox/chromium/sandbox/win/src/target_process.cc b/security/sandbox/chromium/sandbox/win/src/target_process.cc index c4073a7ea5bf..4661267d1e51 100644 --- a/security/sandbox/chromium/sandbox/win/src/target_process.cc +++ b/security/sandbox/chromium/sandbox/win/src/target_process.cc @@ -291,15 +291,6 @@ ResultCode TargetProcess::Init(Dispatcher* ipc_dispatcher, return SBOX_ERROR_CREATE_FILE_MAPPING; } - DWORD access = FILE_MAP_READ | FILE_MAP_WRITE | SECTION_QUERY; - HANDLE target_shared_section; - if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_.Get(), - sandbox_process_info_.process_handle(), - &target_shared_section, access, false, 0)) { - *win_error = ::GetLastError(); - return SBOX_ERROR_DUPLICATE_SHARED_SECTION; - } - void* shared_memory = ::MapViewOfFile( shared_section_.Get(), FILE_MAP_WRITE | FILE_MAP_READ, 0, 0, 0); if (!shared_memory) { @@ -312,14 +303,6 @@ ResultCode TargetProcess::Init(Dispatcher* ipc_dispatcher, ResultCode ret; // Set the global variables in the target. These are not used on the broker. - g_shared_section = target_shared_section; - ret = TransferVariable("g_shared_section", &g_shared_section, - sizeof(g_shared_section)); - g_shared_section = nullptr; - if (SBOX_ALL_OK != ret) { - *win_error = ::GetLastError(); - return ret; - } g_shared_IPC_size = shared_IPC_size; ret = TransferVariable("g_shared_IPC_size", &g_shared_IPC_size, sizeof(g_shared_IPC_size)); @@ -344,6 +327,24 @@ ResultCode TargetProcess::Init(Dispatcher* ipc_dispatcher, if (!ipc_server_->Init(shared_memory, shared_IPC_size, kIPCChannelSize)) return SBOX_ERROR_NO_SPACE; + DWORD access = FILE_MAP_READ | FILE_MAP_WRITE | SECTION_QUERY; + HANDLE target_shared_section; + if (!::DuplicateHandle(::GetCurrentProcess(), shared_section_.Get(), + sandbox_process_info_.process_handle(), + &target_shared_section, access, false, 0)) { + *win_error = ::GetLastError(); + return SBOX_ERROR_DUPLICATE_SHARED_SECTION; + } + + g_shared_section = target_shared_section; + ret = TransferVariable("g_shared_section", &g_shared_section, + sizeof(g_shared_section)); + g_shared_section = nullptr; + if (SBOX_ALL_OK != ret) { + *win_error = ::GetLastError(); + return ret; + } + // After this point we cannot use this handle anymore. ::CloseHandle(sandbox_process_info_.TakeThreadHandle()); diff --git a/security/sandbox/chromium/sandbox/win/src/win_utils.cc b/security/sandbox/chromium/sandbox/win/src/win_utils.cc index 0c97c31bcd0c..8fb345a239e7 100644 --- a/security/sandbox/chromium/sandbox/win/src/win_utils.cc +++ b/security/sandbox/chromium/sandbox/win/src/win_utils.cc @@ -199,6 +199,7 @@ bool ResolveRegistryName(std::wstring name, std::wstring* resolved_name) { // \??\c:\some\foo\bar // \Device\HarddiskVolume0\some\foo\bar // \??\HarddiskVolume0\some\foo\bar +// \??\UNC\SERVER\Share\some\foo\bar DWORD IsReparsePoint(const std::wstring& full_path) { // Check if it's a pipe. We can't query the attributes of a pipe. if (IsPipe(full_path)) @@ -212,28 +213,33 @@ DWORD IsReparsePoint(const std::wstring& full_path) { if (!has_drive && !is_device_path && !nt_path) return ERROR_INVALID_NAME; - bool added_implied_device = false; if (!has_drive) { - path = std::wstring(kNTDotPrefix) + path; - added_implied_device = true; + // Add Win32 device namespace prefix, required for some Windows APIs. + path.insert(0, kNTDotPrefix); } - std::wstring::size_type last_pos = std::wstring::npos; - bool passed_once = false; + // Ensure that volume path matches start of path. + wchar_t vol_path[MAX_PATH]; + if (!::GetVolumePathNameW(path.c_str(), vol_path, MAX_PATH)) { + // This will fail if this is a device that isn't volume related, which can't + // then be a reparse point. + return is_device_path ? ERROR_NOT_A_REPARSE_POINT : ERROR_INVALID_NAME; + } + + // vol_path includes a trailing slash, so reduce size for path and loop check. + size_t vol_path_len = wcslen(vol_path) - 1; + if (!EqualPath(path, vol_path, vol_path_len)) { + return ERROR_INVALID_NAME; + } do { - path = path.substr(0, last_pos); - DWORD attributes = ::GetFileAttributes(path.c_str()); if (INVALID_FILE_ATTRIBUTES == attributes) { DWORD error = ::GetLastError(); if (error != ERROR_FILE_NOT_FOUND && error != ERROR_PATH_NOT_FOUND && + error != ERROR_INVALID_FUNCTION && error != ERROR_INVALID_NAME) { // Unexpected error. - if (passed_once && added_implied_device && - (path.rfind(L'\\') == kNTDotPrefixLen - 1)) { - break; - } return error; } } else if (FILE_ATTRIBUTE_REPARSE_POINT & attributes) { @@ -241,9 +247,8 @@ DWORD IsReparsePoint(const std::wstring& full_path) { return ERROR_SUCCESS; } - passed_once = true; - last_pos = path.rfind(L'\\'); - } while (last_pos > 2); // Skip root dir. + path.resize(path.rfind(L'\\')); + } while (path.size() > vol_path_len); // Skip root dir. return ERROR_NOT_A_REPARSE_POINT; } @@ -263,11 +268,11 @@ bool SameObject(HANDLE handle, const wchar_t* full_path) { DCHECK_NT(!path.empty()); // This may end with a backslash. - const wchar_t kBackslash = '\\'; - if (path.back() == kBackslash) - path = path.substr(0, path.length() - 1); + if (path.back() == L'\\') { + path.pop_back(); + } - // Perfect match (case-insesitive check). + // Perfect match (case-insensitive check). if (EqualPath(actual_path, path)) return true; @@ -276,40 +281,44 @@ bool SameObject(HANDLE handle, const wchar_t* full_path) { if (!has_drive && nt_path) { std::wstring simple_actual_path; - if (!IsDevicePath(actual_path, &simple_actual_path)) - return false; - - // Perfect match (case-insesitive check). - return (EqualPath(simple_actual_path, path)); + if (IsDevicePath(path, &path)) { + if (IsDevicePath(actual_path, &simple_actual_path)) { + // Perfect match (case-insensitive check). + return (EqualPath(simple_actual_path, path)); + } else { + return false; + } + } else { + // Add Win32 device namespace for GetVolumePathName. + path.insert(0, kNTDotPrefix); + } } - if (!has_drive) + // Get the volume path in the same format as actual_path. + wchar_t vol_path[MAX_PATH]; + if (!::GetVolumePathName(path.c_str(), vol_path, MAX_PATH)) { return false; - - // We only need 3 chars, but let's alloc a buffer for four. - wchar_t drive[4] = {0}; - wchar_t vol_name[MAX_PATH]; - memcpy(drive, &path[0], 2 * sizeof(*drive)); - - // We'll get a double null terminated string. - DWORD vol_length = ::QueryDosDeviceW(drive, vol_name, MAX_PATH); - if (vol_length < 2 || vol_length == MAX_PATH) + } + size_t vol_path_len = wcslen(vol_path); + base::string16 nt_vol; + if (!GetNtPathFromWin32Path(vol_path, &nt_vol)) { return false; - - // Ignore the nulls at the end. - vol_length = static_cast(wcslen(vol_name)); + } // The two paths should be the same length. - if (vol_length + path.size() - 2 != actual_path.size()) + if (nt_vol.size() + path.size() - vol_path_len != actual_path.size()) { return false; + } - // Check up to the drive letter. - if (!EqualPath(actual_path, vol_name, vol_length)) + // Check the volume matches. + if (!EqualPath(actual_path, nt_vol.c_str(), nt_vol.size())) { return false; + } - // Check the path after the drive letter. - if (!EqualPath(actual_path, vol_length, path, 2)) + // Check the path after the volume matches. + if (!EqualPath(actual_path, nt_vol.size(), path, vol_path_len)) { return false; + } return true; } @@ -452,10 +461,11 @@ bool GetNtPathFromWin32Path(const std::wstring& path, std::wstring* nt_path) { bool WriteProtectedChildMemory(HANDLE child_process, void* address, const void* buffer, - size_t length) { + size_t length, + DWORD writeProtection) { // First, remove the protections. DWORD old_protection; - if (!::VirtualProtectEx(child_process, address, length, PAGE_WRITECOPY, + if (!::VirtualProtectEx(child_process, address, length, writeProtection, &old_protection)) return false; @@ -540,6 +550,30 @@ void* GetProcessBaseAddress(HANDLE process) { if (magic[0] != 'M' || magic[1] != 'Z') return nullptr; +#if defined(_M_ARM64) + // Windows 10 on ARM64 has multi-threaded DLL loading that does not work with + // the sandbox. (On x86 this gets disabled by hook detection code that was not + // ported to ARM64). This overwrites the LoaderThreads value in the process + // parameters part of the PEB, if it is set to the default of 0 (which + // actually means it defaults to 4 loading threads). This is an undocumented + // field so there is a, probably small, risk that it might change or move in + // the future. In order to slightly guard against that we only update if the + // value is currently 0. + auto processParameters = reinterpret_cast(peb.ProcessParameters); + const uint32_t loaderThreadsOffset = 0x40c; + uint32_t maxLoaderThreads = 0; + BOOL memoryRead = ::ReadProcessMemory( + process, processParameters + loaderThreadsOffset, &maxLoaderThreads, + sizeof(maxLoaderThreads), &bytes_read); + if (memoryRead && (sizeof(maxLoaderThreads) == bytes_read) && + (maxLoaderThreads == 0)) { + maxLoaderThreads = 1; + WriteProtectedChildMemory(process, processParameters + loaderThreadsOffset, + &maxLoaderThreads, sizeof(maxLoaderThreads), + PAGE_READWRITE); + } +#endif + return base_address; } diff --git a/security/sandbox/chromium/sandbox/win/src/win_utils.h b/security/sandbox/chromium/sandbox/win/src/win_utils.h index 2e79eeb78bbf..7c848f2f08c4 100644 --- a/security/sandbox/chromium/sandbox/win/src/win_utils.h +++ b/security/sandbox/chromium/sandbox/win/src/win_utils.h @@ -116,7 +116,8 @@ bool ResolveRegistryName(std::wstring name, std::wstring* resolved_name); bool WriteProtectedChildMemory(HANDLE child_process, void* address, const void* buffer, - size_t length); + size_t length, + DWORD writeProtection = PAGE_WRITECOPY); // Allocates |buffer_bytes| in child (PAGE_READWRITE) and copies data // from |local_buffer| in this process into |child|. |remote_buffer|