diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-08-30 10:22:43 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2019-08-30 12:36:28 +0000 |
commit | 271a6c3487a14599023a9106329505597638d793 (patch) | |
tree | e040d58ffc86c1480b79ca8528020ca9ec919bf8 /chromium/sandbox | |
parent | 7b2ffa587235a47d4094787d72f38102089f402a (diff) | |
download | qtwebengine-chromium-271a6c3487a14599023a9106329505597638d793.tar.gz |
BASELINE: Update Chromium to 77.0.3865.59
Change-Id: I1e89a5f3b009a9519a6705102ad65c92fe736f21
Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
Diffstat (limited to 'chromium/sandbox')
44 files changed, 924 insertions, 345 deletions
diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc index 479d1ed55a3..806d13c1a84 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc @@ -11,6 +11,7 @@ #include <sys/types.h> #include <unistd.h> +#include "base/clang_coverage_buildflags.h" #include "base/logging.h" #include "build/build_config.h" #include "sandbox/linux/bpf_dsl/bpf_dsl.h" @@ -127,6 +128,16 @@ ResultExpr EvaluateSyscallImpl(int fs_denied_errno, #endif // defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) || // defined(MEMORY_SANITIZER) +#if BUILDFLAG(CLANG_COVERAGE) + if (SyscallSets::IsPrctl(sysno)) { + return Allow(); + } + + if (sysno == __NR_ftruncate) { + return Allow(); + } +#endif + if (IsBaselinePolicyAllowed(sysno)) { return Allow(); } diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc index 40fcebf9336..060181bd42f 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/baseline_policy_unittest.cc @@ -24,6 +24,7 @@ #include <time.h> #include <unistd.h> +#include "base/clang_coverage_buildflags.h" #include "base/files/scoped_file.h" #include "base/macros.h" #include "base/posix/eintr_wrapper.h" @@ -342,6 +343,7 @@ BPF_TEST_C(BaselinePolicy, PrctlDumpable, BaselinePolicy) { #define PR_CAPBSET_READ 23 #endif +#if !BUILDFLAG(CLANG_COVERAGE) BPF_DEATH_TEST_C(BaselinePolicy, PrctlSigsys, DEATH_SEGV_MESSAGE(GetPrctlErrorMessageContentForTests()), @@ -349,6 +351,7 @@ BPF_DEATH_TEST_C(BaselinePolicy, prctl(PR_CAPBSET_READ, 0, 0, 0, 0); _exit(1); } +#endif BPF_TEST_C(BaselinePolicy, GetOrSetPriority, BaselinePolicy) { errno = 0; diff --git a/chromium/sandbox/win/BUILD.gn b/chromium/sandbox/win/BUILD.gn index 2acc1c9a2f7..63864dcfef6 100644 --- a/chromium/sandbox/win/BUILD.gn +++ b/chromium/sandbox/win/BUILD.gn @@ -118,6 +118,12 @@ static_library("sandbox") { "src/sharedmem_ipc_server.h", "src/sid.cc", "src/sid.h", + "src/signed_dispatcher.cc", + "src/signed_dispatcher.h", + "src/signed_interception.cc", + "src/signed_interception.h", + "src/signed_policy.cc", + "src/signed_policy.h", "src/sync_dispatcher.cc", "src/sync_dispatcher.h", "src/sync_interception.cc", diff --git a/chromium/sandbox/win/src/crosscall_client.h b/chromium/sandbox/win/src/crosscall_client.h index 7bcfb3cfd94..98cd65dc55f 100644 --- a/chromium/sandbox/win/src/crosscall_client.h +++ b/chromium/sandbox/win/src/crosscall_client.h @@ -224,7 +224,7 @@ class CopyHelper<InOutCountedBuffer> { // We are touching user memory, this has to be done from inside a try // except. __try { - memcpy(t_.Buffer(), buffer, t_.Size()); + memcpy_wrapper(t_.Buffer(), buffer, t_.Size()); } __except (EXCEPTION_EXECUTE_HANDLER) { return false; } diff --git a/chromium/sandbox/win/src/crosscall_params.h b/chromium/sandbox/win/src/crosscall_params.h index 4dccd2b7ff4..60a1bb60aa0 100644 --- a/chromium/sandbox/win/src/crosscall_params.h +++ b/chromium/sandbox/win/src/crosscall_params.h @@ -20,15 +20,11 @@ #include "base/macros.h" #include "sandbox/win/src/internal_types.h" +#if !defined(SANDBOX_FUZZ_TARGET) +#include "sandbox/win/src/sandbox_nt_types.h" +#endif #include "sandbox/win/src/sandbox_types.h" -// Increases |value| until there is no need for padding given an int64_t -// alignment. Returns the increased value. -inline uint32_t Align(uint32_t value) { - uint32_t alignment = sizeof(int64_t); - return ((value + alignment - 1) / alignment) * alignment; -} - // This header is part of CrossCall: the sandbox inter-process communication. // This header defines the basic types used both in the client IPC and in the // server IPC code. CrossCallParams and ActualCallParams model the input @@ -49,6 +45,26 @@ inline uint32_t Align(uint32_t value) { namespace sandbox { +// This is the list of all imported symbols from ntdll.dll. +SANDBOX_INTERCEPT NtExports g_nt; + +namespace { + +// Increases |value| until there is no need for padding given an int64_t +// alignment. Returns the increased value. +inline uint32_t Align(uint32_t value) { + uint32_t alignment = sizeof(int64_t); + return ((value + alignment - 1) / alignment) * alignment; +} + +inline void* memcpy_wrapper(void* dest, const void* src, size_t count) { + if (g_nt.memcpy) + return g_nt.memcpy(dest, src, count); + return memcpy(dest, src, count); +} + +} // namespace + // max number of extended return parameters. See CrossCallReturn const size_t kExtendedReturnCount = 8; @@ -243,7 +259,7 @@ class ActualCallParams : public CrossCallParams { // We might be touching user memory, this has to be done from inside a try // except. __try { - memcpy(dest, parameter_address, size); + memcpy_wrapper(dest, parameter_address, size); } __except (EXCEPTION_EXECUTE_HANDLER) { return false; } diff --git a/chromium/sandbox/win/src/handle_closer.cc b/chromium/sandbox/win/src/handle_closer.cc index 3e2f1f00932..23fe0c80b25 100644 --- a/chromium/sandbox/win/src/handle_closer.cc +++ b/chromium/sandbox/win/src/handle_closer.cc @@ -104,22 +104,10 @@ bool HandleCloser::InitializeTargetHandles(TargetProcess* target) { if (!SetupHandleList(local_buffer.get(), bytes_needed)) return false; - HANDLE child = target->Process(); - - // Allocate memory in the target process without specifying the address - void* remote_data = ::VirtualAllocEx(child, nullptr, bytes_needed, MEM_COMMIT, - PAGE_READWRITE); - if (!remote_data) - return false; - - // Copy the handle buffer over. - SIZE_T bytes_written; - bool result = ::WriteProcessMemory(child, remote_data, local_buffer.get(), - bytes_needed, &bytes_written); - if (!result || bytes_written != bytes_needed) { - ::VirtualFreeEx(child, remote_data, 0, MEM_RELEASE); + void* remote_data; + if (!CopyToChildMemory(target->Process(), local_buffer.get(), bytes_needed, + &remote_data)) return false; - } g_handles_to_close = reinterpret_cast<HandleCloserInfo*>(remote_data); diff --git a/chromium/sandbox/win/src/interception.cc b/chromium/sandbox/win/src/interception.cc index d66ceb2e791..cfe9b0b7ea2 100644 --- a/chromium/sandbox/win/src/interception.cc +++ b/chromium/sandbox/win/src/interception.cc @@ -142,14 +142,12 @@ ResultCode InterceptionManager::InitializeInterceptions() { return SBOX_ERROR_CANNOT_SETUP_INTERCEPTION_CONFIG_BUFFER; void* remote_buffer; - ResultCode rc = - CopyDataToChild(local_buffer.get(), buffer_bytes, &remote_buffer); - - if (rc != SBOX_ALL_OK) - return rc; + if (!CopyToChildMemory(child_->Process(), local_buffer.get(), buffer_bytes, + &remote_buffer)) + return SBOX_ERROR_CANNOT_COPY_DATA_TO_CHILD; bool hot_patch_needed = (0 != buffer_bytes); - rc = PatchNtdll(hot_patch_needed); + ResultCode rc = PatchNtdll(hot_patch_needed); if (rc != SBOX_ALL_OK) return rc; @@ -333,36 +331,6 @@ bool InterceptionManager::SetupInterceptionInfo(const InterceptionData& data, return true; } -ResultCode InterceptionManager::CopyDataToChild(const void* local_buffer, - size_t buffer_bytes, - void** remote_buffer) const { - DCHECK(remote_buffer); - if (0 == buffer_bytes) { - *remote_buffer = nullptr; - return SBOX_ALL_OK; - } - - HANDLE child = child_->Process(); - - // Allocate memory on the target process without specifying the address - void* remote_data = ::VirtualAllocEx(child, nullptr, buffer_bytes, MEM_COMMIT, - PAGE_READWRITE); - if (!remote_data) - return SBOX_ERROR_NO_SPACE; - - SIZE_T bytes_written; - bool success = ::WriteProcessMemory(child, remote_data, local_buffer, - buffer_bytes, &bytes_written); - if (!success || bytes_written != buffer_bytes) { - ::VirtualFreeEx(child, remote_data, 0, MEM_RELEASE); - return SBOX_ERROR_CANNOT_COPY_DATA_TO_CHILD; - } - - *remote_buffer = remote_data; - - return SBOX_ALL_OK; -} - // Only return true if the child should be able to perform this interception. bool InterceptionManager::IsInterceptionPerformedByChild( const InterceptionData& data) const { diff --git a/chromium/sandbox/win/src/interceptors.h b/chromium/sandbox/win/src/interceptors.h index 44b34e37f20..5788614707b 100644 --- a/chromium/sandbox/win/src/interceptors.h +++ b/chromium/sandbox/win/src/interceptors.h @@ -61,6 +61,8 @@ enum InterceptorId { GETOPMRANDOMNUMBER_ID, GETSUGGESTEDOPMPROTECTEDOUTPUTARRAYSIZE_ID, SETOPMSIGNINGKEYANDSEQUENCENUMBERS_ID, + // Signed dispatcher: + CREATE_SECTION_ID, INTERCEPTOR_MAX_ID }; diff --git a/chromium/sandbox/win/src/interceptors_64.cc b/chromium/sandbox/win/src/interceptors_64.cc index 669d68bae49..0ac2671fcc4 100644 --- a/chromium/sandbox/win/src/interceptors_64.cc +++ b/chromium/sandbox/win/src/interceptors_64.cc @@ -13,6 +13,7 @@ #include "sandbox/win/src/registry_interception.h" #include "sandbox/win/src/sandbox_nt_types.h" #include "sandbox/win/src/sandbox_types.h" +#include "sandbox/win/src/signed_interception.h" #include "sandbox/win/src/sync_interception.h" #include "sandbox/win/src/target_interceptions.h" @@ -512,4 +513,19 @@ SANDBOX_INTERCEPT NTSTATUS WINAPI TargetConfigureOPMProtectedOutput64( additional_parameters); } +SANDBOX_INTERCEPT NTSTATUS WINAPI +TargetNtCreateSection64(PHANDLE section_handle, + ACCESS_MASK desired_access, + POBJECT_ATTRIBUTES object_attributes, + PLARGE_INTEGER maximum_size, + ULONG section_page_protection, + ULONG allocation_attributes, + HANDLE file_handle) { + NtCreateSectionFunction orig_fn = + reinterpret_cast<NtCreateSectionFunction>(g_originals[CREATE_SECTION_ID]); + return TargetNtCreateSection( + orig_fn, section_handle, desired_access, object_attributes, maximum_size, + section_page_protection, allocation_attributes, file_handle); +} + } // namespace sandbox diff --git a/chromium/sandbox/win/src/interceptors_64.h b/chromium/sandbox/win/src/interceptors_64.h index 1016b7cfd64..f34627dcd2a 100644 --- a/chromium/sandbox/win/src/interceptors_64.h +++ b/chromium/sandbox/win/src/interceptors_64.h @@ -310,6 +310,19 @@ SANDBOX_INTERCEPT NTSTATUS WINAPI TargetConfigureOPMProtectedOutput64( ULONG additional_parameters_size, const BYTE* additional_parameters); +// ----------------------------------------------------------------------- +// Interceptors handled by the signed process code. + +// Interception of NtCreateSection on the child process. +SANDBOX_INTERCEPT NTSTATUS WINAPI +TargetNtCreateSection64(PHANDLE section_handle, + ACCESS_MASK desired_access, + POBJECT_ATTRIBUTES object_attributes, + PLARGE_INTEGER maximum_size, + ULONG section_page_protection, + ULONG allocation_attributes, + HANDLE file_handle); + } // extern "C" } // namespace sandbox diff --git a/chromium/sandbox/win/src/ipc_tags.h b/chromium/sandbox/win/src/ipc_tags.h index 1c754cdd1fb..2d38cdf3657 100644 --- a/chromium/sandbox/win/src/ipc_tags.h +++ b/chromium/sandbox/win/src/ipc_tags.h @@ -44,6 +44,7 @@ enum { IPC_GDI_GETOPMRANDOMNUMBER_TAG, IPC_GDI_GETSUGGESTEDOPMPROTECTEDOUTPUTARRAYSIZE_TAG, IPC_GDI_SETOPMSIGNINGKEYANDSEQUENCENUMBERS_TAG, + IPC_NTCREATESECTION_TAG, IPC_LAST_TAG }; diff --git a/chromium/sandbox/win/src/named_pipe_dispatcher.cc b/chromium/sandbox/win/src/named_pipe_dispatcher.cc index bc7e3d74920..734a71c1a6e 100644 --- a/chromium/sandbox/win/src/named_pipe_dispatcher.cc +++ b/chromium/sandbox/win/src/named_pipe_dispatcher.cc @@ -77,7 +77,7 @@ bool NamedPipeDispatcher::CreateNamedPipe(IPCInfo* ipc, // http://msdn.microsoft.com/en-us/library/aa365247(VS.85).aspx // This ensures even if there is a path traversal in the pipe name, and it is // able to get past the checks above, it will still not be allowed to escape - // our whitelisted namespace. + // our allowed namespace. if (name->compare(0, 4, L"\\\\.\\") == 0) name->replace(0, 4, L"\\\\\?\\"); diff --git a/chromium/sandbox/win/src/named_pipe_policy_test.cc b/chromium/sandbox/win/src/named_pipe_policy_test.cc index e588eff5776..d338be4627e 100644 --- a/chromium/sandbox/win/src/named_pipe_policy_test.cc +++ b/chromium/sandbox/win/src/named_pipe_policy_test.cc @@ -22,7 +22,7 @@ SBOX_TESTS_COMMAND int NamedPipe_Create(int argc, wchar_t** argv) { if (INVALID_HANDLE_VALUE == pipe) return SBOX_TEST_DENIED; - // The second parameter allows us to enforce a whitelist for where the + // The second parameter allows us to enforce an allowlist for where the // pipe should be in the object namespace after creation. if (argc == 2) { base::string16 handle_name; diff --git a/chromium/sandbox/win/src/nt_internals.h b/chromium/sandbox/win/src/nt_internals.h index 2b2dc0f2889..60415664369 100644 --- a/chromium/sandbox/win/src/nt_internals.h +++ b/chromium/sandbox/win/src/nt_internals.h @@ -13,11 +13,14 @@ typedef LONG NTSTATUS; #define NT_SUCCESS(st) (st >= 0) +#define NT_ERROR(st) ((((ULONG)(st)) >> 30) == 3) +// clang-format off #define STATUS_SUCCESS ((NTSTATUS)0x00000000L) #define STATUS_BUFFER_OVERFLOW ((NTSTATUS)0x80000005L) #define STATUS_UNSUCCESSFUL ((NTSTATUS)0xC0000001L) #define STATUS_NOT_IMPLEMENTED ((NTSTATUS)0xC0000002L) +#define STATUS_INVALID_INFO_CLASS ((NTSTATUS)0xC0000003L) #define STATUS_INFO_LENGTH_MISMATCH ((NTSTATUS)0xC0000004L) #ifndef STATUS_INVALID_PARAMETER // It is now defined in Windows 2008 SDK. @@ -32,6 +35,8 @@ typedef LONG NTSTATUS; #define STATUS_INVALID_IMAGE_FORMAT ((NTSTATUS)0xC000007BL) #define STATUS_NO_TOKEN ((NTSTATUS)0xC000007CL) #define STATUS_NOT_SUPPORTED ((NTSTATUS)0xC00000BBL) +#define STATUS_INVALID_IMAGE_HASH ((NTSTATUS)0xC0000428L) +// clang-format on #define CURRENT_PROCESS ((HANDLE)-1) #define CURRENT_THREAD ((HANDLE)-2) @@ -311,7 +316,17 @@ typedef enum _PROCESSINFOCLASS { ProcessExecuteFlags = 0x22 } PROCESSINFOCLASS; -// Partial definition only. +// For the structure documentation, see +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa813741(v=vs.85).aspx +typedef struct _RTL_USER_PROCESS_PARAMETERS { + BYTE Reserved1[16]; + PVOID Reserved2[10]; + UNICODE_STRING ImagePathName; + UNICODE_STRING CommandLine; +} RTL_USER_PROCESS_PARAMETERS, *PRTL_USER_PROCESS_PARAMETERS; + +// Partial definition only, from +// https://msdn.microsoft.com/en-us/library/windows/desktop/aa813706(v=vs.85).aspx typedef struct _PEB { BYTE InheritedAddressSpace; BYTE ReadImageFileExecOptions; @@ -319,6 +334,8 @@ typedef struct _PEB { BYTE SpareBool; PVOID Mutant; PVOID ImageBaseAddress; + PVOID Ldr; + PRTL_USER_PROCESS_PARAMETERS ProcessParameters; } PEB, *PPEB; typedef LONG KPRIORITY; @@ -695,6 +712,11 @@ typedef NTSTATUS(WINAPI* NtSignalAndWaitForSingleObjectFunction)( IN BOOLEAN Alertable, IN PLARGE_INTEGER Timeout OPTIONAL); +typedef NTSTATUS(WINAPI* NtWaitForSingleObjectFunction)( + IN HANDLE ObjectHandle, + IN BOOLEAN Alertable, + IN PLARGE_INTEGER TimeOut OPTIONAL); + typedef NTSTATUS(WINAPI* NtQuerySystemInformation)( IN SYSTEM_INFORMATION_CLASS SystemInformationClass, OUT PVOID SystemInformation, diff --git a/chromium/sandbox/win/src/policy_broker.cc b/chromium/sandbox/win/src/policy_broker.cc index 679eb7db39d..406057acbbf 100644 --- a/chromium/sandbox/win/src/policy_broker.cc +++ b/chromium/sandbox/win/src/policy_broker.cc @@ -55,6 +55,8 @@ bool InitGlobalNt() { INIT_GLOBAL_NT(QuerySection); INIT_GLOBAL_NT(QueryVirtualMemory); INIT_GLOBAL_NT(UnmapViewOfSection); + INIT_GLOBAL_NT(SignalAndWaitForSingleObject); + INIT_GLOBAL_NT(WaitForSingleObject); INIT_GLOBAL_RTL(RtlAllocateHeap); INIT_GLOBAL_RTL(RtlAnsiStringToUnicodeString); diff --git a/chromium/sandbox/win/src/policy_target_test.cc b/chromium/sandbox/win/src/policy_target_test.cc index 39b5afb4e82..e69ff4f5898 100644 --- a/chromium/sandbox/win/src/policy_target_test.cc +++ b/chromium/sandbox/win/src/policy_target_test.cc @@ -2,7 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/memory/shared_memory.h" +#include "base/memory/read_only_shared_memory_region.h" +#include "base/memory/writable_shared_memory_region.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" #include "base/win/scoped_process_information.h" @@ -404,31 +405,30 @@ TEST(PolicyTargetTest, ShareHandleTest) { ASSERT_TRUE(broker); base::StringPiece contents = "Hello World"; - std::string name = "TestSharedMemory"; - base::SharedMemoryCreateOptions options; - options.size = contents.size(); - options.share_read_only = true; - options.name_deprecated = &name; - base::SharedMemory writable_shmem; - ASSERT_TRUE(writable_shmem.Create(options)); - ASSERT_TRUE(writable_shmem.Map(options.size)); - memcpy(writable_shmem.memory(), contents.data(), contents.size()); - - base::SharedMemory read_only_view; - ASSERT_TRUE(read_only_view.Open(name, true)); + base::WritableSharedMemoryRegion writable_region = + base::WritableSharedMemoryRegion::Create(contents.size()); + ASSERT_TRUE(writable_region.IsValid()); + base::WritableSharedMemoryMapping writable_mapping = writable_region.Map(); + ASSERT_TRUE(writable_mapping.IsValid()); + memcpy(writable_mapping.memory(), contents.data(), contents.size()); // Get the path to the sandboxed app. wchar_t prog_name[MAX_PATH]; GetModuleFileNameW(nullptr, prog_name, MAX_PATH); + base::ReadOnlySharedMemoryRegion read_only_region = + base::WritableSharedMemoryRegion::ConvertToReadOnly( + std::move(writable_region)); + ASSERT_TRUE(read_only_region.IsValid()); + scoped_refptr<TargetPolicy> policy = broker->CreatePolicy(); - policy->AddHandleToShare(read_only_view.handle().GetHandle()); + policy->AddHandleToShare(read_only_region.GetPlatformHandle()); base::string16 arguments(L"\""); arguments += prog_name; arguments += L"\" -child 0 shared_memory_handle "; arguments += base::NumberToString16( - base::win::HandleToUint32(read_only_view.handle().GetHandle())); + base::win::HandleToUint32(read_only_region.GetPlatformHandle())); // Launch the app. ResultCode result = SBOX_ALL_OK; diff --git a/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc b/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc index df1c24dd443..59db0b00848 100644 --- a/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc +++ b/chromium/sandbox/win/src/process_mitigations_dyncode_unittest.cc @@ -447,17 +447,12 @@ TEST(ProcessMitigationsTest, CheckWin81DynamicCode_BaseCase) { if (base::win::GetVersion() < base::win::Version::WIN8_1) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); // Expect success, no mitigation. - DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE, true, false); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE, + true /* expect_success */, + false /* enable_mitigation */); } // This test validates that setting the MITIGATION_DYNAMIC_CODE_DISABLE @@ -466,17 +461,12 @@ TEST(ProcessMitigationsTest, CheckWin81DynamicCode_TestMitigation) { if (base::win::GetVersion() < base::win::Version::WIN8_1) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); // Expect failure, with mitigation. - DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE, false, true); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE, + false /* expect_success */, + true /* enable_mitigation */); } //------------------------------------------------------------------------------ @@ -534,18 +524,13 @@ TEST(ProcessMitigationsTest, CheckWin10DynamicCodeOptOut_BaseCase) { if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); // Expect success, no mitigation (and therefore no thread opt-out). DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE_WITH_OPT_OUT, - true, false, false); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + true /* expect_success */, + false /* enable_mitigation */, + false /* with_thread_opt_out */); } // This test validates that setting the @@ -555,18 +540,13 @@ TEST(ProcessMitigationsTest, CheckWin10DynamicCodeOptOut_TestMitigation) { if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); // Expect failure, with mitigation, no thread opt-out. DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE_WITH_OPT_OUT, - false, true, false); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + false /* expect_success */, + true /* enable_mitigation */, + false /* with_thread_opt_out */); } // This test validates that setting the @@ -577,18 +557,13 @@ TEST(ProcessMitigationsTest, if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); // Expect success, with mitigation, with thread opt-out. DynamicCodeTestHarness(sandbox::MITIGATION_DYNAMIC_CODE_DISABLE_WITH_OPT_OUT, - true, true, true); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + true /* expect_success */, + true /* enable_mitigation */, + true /* with_thread_opt_out */); } } // namespace sandbox diff --git a/chromium/sandbox/win/src/process_mitigations_extensionpoints_unittest.cc b/chromium/sandbox/win/src/process_mitigations_extensionpoints_unittest.cc index 3c4234fd4ac..a9da035be55 100644 --- a/chromium/sandbox/win/src/process_mitigations_extensionpoints_unittest.cc +++ b/chromium/sandbox/win/src/process_mitigations_extensionpoints_unittest.cc @@ -402,16 +402,10 @@ TEST(ProcessMitigationsTest, if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // (is_success_test, global_hook) - TestWin8ExtensionPointHookWrapper(true, true); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointHookWrapper(true /* is_success_test */, + true /* global hook */); } // This test validates that setting the MITIGATION_EXTENSION_POINT_DISABLE @@ -423,16 +417,10 @@ TEST(ProcessMitigationsTest, if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // (is_success_test, global_hook) - TestWin8ExtensionPointHookWrapper(false, true); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointHookWrapper(false /* is_success_test */, + true /* global hook */); } // This test validates that a "legitimate" hook CAN be set on the sandboxed @@ -443,16 +431,10 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_Hook_Success) { if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // (is_success_test, global_hook) - TestWin8ExtensionPointHookWrapper(true, false); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointHookWrapper(true /* is_success_test */, + false /* global hook */); } // *** Important: MITIGATION_EXTENSION_POINT_DISABLE does NOT prevent @@ -466,16 +448,10 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_Hook_Failure) { if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // (is_success_test, global_hook) - TestWin8ExtensionPointHookWrapper(false, false); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointHookWrapper(false /* is_success_test */, + false /* global hook */); } // This test validates that an AppInit Dll CAN be added to a target @@ -487,15 +463,9 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_AppInit_Success) { if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - TestWin8ExtensionPointAppInitWrapper(true); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointAppInitWrapper(true /* is_success_test */); } // This test validates that setting the MITIGATION_EXTENSION_POINT_DISABLE @@ -507,15 +477,9 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin8ExtensionPoint_AppInit_Failure) { if (base::win::GetVersion() < base::win::Version::WIN8) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_extension_point_test_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - TestWin8ExtensionPointAppInitWrapper(false); + ScopedTestMutex mutex(g_extension_point_test_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin8ExtensionPointAppInitWrapper(false /* is_success_test */); } } // namespace sandbox diff --git a/chromium/sandbox/win/src/process_mitigations_imageload_unittest.cc b/chromium/sandbox/win/src/process_mitigations_imageload_unittest.cc index f6af3d8a459..cece2450cfd 100644 --- a/chromium/sandbox/win/src/process_mitigations_imageload_unittest.cc +++ b/chromium/sandbox/win/src/process_mitigations_imageload_unittest.cc @@ -302,7 +302,7 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteSuccess) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - TestWin10ImageLoadRemote(true); + TestWin10ImageLoadRemote(true /* is_success_test */); } // This test validates that setting the MITIGATION_IMAGE_LOAD_NO_REMOTE @@ -314,7 +314,7 @@ TEST(ProcessMitigationsTest, DISABLED_CheckWin10ImageLoadNoRemoteFailure) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - TestWin10ImageLoadRemote(false); + TestWin10ImageLoadRemote(false /* is_success_test */); } //------------------------------------------------------------------------------ @@ -361,7 +361,7 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelSuccess) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - TestWin10ImageLoadLowLabel(true); + TestWin10ImageLoadLowLabel(true /* is_success_test */); } // This test validates that setting the MITIGATION_IMAGE_LOAD_NO_LOW_LABEL @@ -370,7 +370,7 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadNoLowLabelFailure) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - TestWin10ImageLoadLowLabel(false); + TestWin10ImageLoadLowLabel(false /* is_success_test */); } //------------------------------------------------------------------------------ @@ -421,16 +421,11 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Baseline) { if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_hijack_dlls_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(g_hijack_dlls_mutex); // Baseline test, and expect the DLL to NOT be in system32. - TestWin10ImageLoadPreferSys32(true, false); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10ImageLoadPreferSys32(true /* baseline_test */, + false /* expect_sys32_path */); } // This test validates that import hijacking succeeds, if the @@ -442,16 +437,11 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Success) { if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_hijack_dlls_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(g_hijack_dlls_mutex); // Not a baseline test, and expect the DLL to be in system32. - TestWin10ImageLoadPreferSys32(false, true); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10ImageLoadPreferSys32(false /* baseline_test */, + true /* expect_sys32_path */); } // This test validates that setting the MITIGATION_IMAGE_LOAD_PREFER_SYS32 @@ -462,16 +452,11 @@ TEST(ProcessMitigationsTest, CheckWin10ImageLoadPreferSys32_Failure) { if (base::win::GetVersion() < base::win::Version::WIN10_RS1) return; - HANDLE mutex = ::CreateMutexW(nullptr, false, g_hijack_dlls_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(g_hijack_dlls_mutex); // Not a baseline test, and expect the DLL to NOT be in system32. - TestWin10ImageLoadPreferSys32(false, false); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10ImageLoadPreferSys32(false /* baseline_test */, + false /* expect_sys32_path */); } } // namespace sandbox diff --git a/chromium/sandbox/win/src/process_mitigations_unittest.cc b/chromium/sandbox/win/src/process_mitigations_unittest.cc index a0cd2790f85..2062f39d4ee 100644 --- a/chromium/sandbox/win/src/process_mitigations_unittest.cc +++ b/chromium/sandbox/win/src/process_mitigations_unittest.cc @@ -80,17 +80,26 @@ void TestWin10NonSystemFont(bool is_success_test) { // // Trigger test child process (with or without mitigation enabled). //------------------------------------------------------------------------------ -void TestWin10MsSigned(bool expect_success, +void TestWin10MsSigned(int expected, bool enable_mitigation, - bool use_ms_signed_binary) { + bool delayed, + bool use_ms_signed_binary, + bool add_dll_permission, + bool add_directory_permission) { sandbox::TestRunner runner; sandbox::TargetPolicy* policy = runner.GetPolicy(); if (enable_mitigation) { // Enable the ForceMsSigned mitigation. - EXPECT_EQ(policy->SetDelayedProcessMitigations( - sandbox::MITIGATION_FORCE_MS_SIGNED_BINS), - sandbox::SBOX_ALL_OK); + if (delayed) { + EXPECT_EQ(policy->SetDelayedProcessMitigations( + sandbox::MITIGATION_FORCE_MS_SIGNED_BINS), + sandbox::SBOX_ALL_OK); + } else { + EXPECT_EQ(policy->SetProcessMitigations( + sandbox::MITIGATION_FORCE_MS_SIGNED_BINS), + sandbox::SBOX_ALL_OK); + } } // Choose the appropriate DLL and make sure the sandbox allows access to it. @@ -101,21 +110,34 @@ void TestWin10MsSigned(bool expect_success, } else { EXPECT_TRUE(base::PathService::Get(base::DIR_EXE, &dll_path)); dll_path = dll_path.Append(hooking_dll::g_hook_dll_file); + + if (add_dll_permission) { + EXPECT_EQ(sandbox::SBOX_ALL_OK, + policy->AddRule(sandbox::TargetPolicy::SUBSYS_SIGNED_BINARY, + sandbox::TargetPolicy::SIGNED_ALLOW_LOAD, + dll_path.value().c_str())); + } + if (add_directory_permission) { + base::FilePath exe_path; + EXPECT_TRUE(base::PathService::Get(base::DIR_EXE, &exe_path)); + EXPECT_EQ(sandbox::SBOX_ALL_OK, + policy->AddRule( + sandbox::TargetPolicy::SUBSYS_SIGNED_BINARY, + sandbox::TargetPolicy::SIGNED_ALLOW_LOAD, + exe_path.DirName().AppendASCII("*.dll").value().c_str())); + } } EXPECT_TRUE(runner.AddFsRule(sandbox::TargetPolicy::FILES_ALLOW_READONLY, dll_path.value().c_str())); - // Set up test string. base::string16 test = L"TestDllLoad \""; test += dll_path.value().c_str(); test += L"\""; // Note: ERROR_INVALID_IMAGE_HASH is being displayed in a system pop-up when - // the DLL load is attempted, but the value returned from the test - // process itself is SBOX_TEST_FAILED. - EXPECT_EQ((expect_success ? sandbox::SBOX_TEST_SUCCEEDED - : sandbox::SBOX_TEST_FAILED), - runner.RunTest(test.c_str())); + // the DLL load is attempted for delayed mitigations, but the value + // returned from the test process itself is SBOX_TEST_FAILED. + EXPECT_EQ(expected, runner.RunTest(test.c_str())); } } // namespace @@ -689,7 +711,7 @@ TEST(ProcessMitigationsTest, CheckWin10NonSystemFontLockDownLoadSuccess) { if (base::win::GetVersion() < base::win::Version::WIN10) return; - TestWin10NonSystemFont(true); + TestWin10NonSystemFont(true /* is_success_test */); } // This test validates that setting the MITIGATION_NON_SYSTEM_FONTS_DISABLE @@ -698,7 +720,7 @@ TEST(ProcessMitigationsTest, CheckWin10NonSystemFontLockDownLoadFailure) { if (base::win::GetVersion() < base::win::Version::WIN10) return; - TestWin10NonSystemFont(false); + TestWin10NonSystemFont(false /* is_success_test */); } //------------------------------------------------------------------------------ @@ -711,7 +733,7 @@ TEST(ProcessMitigationsTest, CheckWin10NonSystemFontLockDownLoadFailure) { // This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS // mitigation enables the setting on a process. -TEST(ProcessMitigationsTest, CheckWin10MsSignedPolicySuccess) { +TEST(ProcessMitigationsTest, CheckWin10MsSignedPolicySuccessDelayed) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; @@ -736,23 +758,55 @@ TEST(ProcessMitigationsTest, CheckWin10MsSignedPolicySuccess) { #endif // !defined(COMPONENT_BUILD) } +// This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS +// mitigation enables the setting on a process when non-delayed. +TEST(ProcessMitigationsTest, CheckWin10MsSignedPolicySuccess) { + if (base::win::GetVersion() < base::win::Version::WIN10_TH2) + return; + + base::string16 test_command = L"CheckPolicy "; + test_command += std::to_wstring(TESTPOLICY_MSSIGNED); + + //--------------------------------- + // 1) Test setting post-startup. + // **Only test if NOT component build, otherwise component DLLs are not signed + // by MS and prevent process setup. + // **Only test post-startup, otherwise this test executable has dependencies + // on DLLs that are not signed by MS and they prevent process startup. + //--------------------------------- + TestRunner runner2; + sandbox::TargetPolicy* policy = runner2.GetPolicy(); + + EXPECT_EQ(policy->SetProcessMitigations(MITIGATION_FORCE_MS_SIGNED_BINS), + SBOX_ALL_OK); + // In a component build, the DLLs must be allowed to load. +#if defined(COMPONENT_BUILD) + base::FilePath exe_path; + EXPECT_TRUE(base::PathService::Get(base::DIR_EXE, &exe_path)); + // Allow all *.dll in current directory to load. + EXPECT_EQ( + sandbox::SBOX_ALL_OK, + policy->AddRule(sandbox::TargetPolicy::SUBSYS_SIGNED_BINARY, + sandbox::TargetPolicy::SIGNED_ALLOW_LOAD, + exe_path.DirName().AppendASCII("*.dll").value().c_str())); +#endif // defined(COMPONENT_BUILD) + + EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner2.RunTest(test_command.c_str())); +} // This test validates that we can load an unsigned DLL if the // MITIGATION_FORCE_MS_SIGNED_BINS mitigation is NOT set. TEST(ProcessMitigationsTest, CheckWin10MsSigned_Success) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // Expect success; Do not enable mitigation; Use non MS-signed binary. - TestWin10MsSigned(true, false, false); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10MsSigned(sandbox::SBOX_TEST_SUCCEEDED /* expected */, + false /* enable_mitigation */, + false /* delayed */, + false /* use_ms_signed_binary */, + false /* add_dll_permission */, + false /* add_directory_permission */); } // This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS @@ -761,17 +815,67 @@ TEST(ProcessMitigationsTest, CheckWin10MsSigned_Failure) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); + + TestWin10MsSigned(sandbox::SBOX_TEST_FAILED /* expected */, + true /* enable_mitigation */, + true /* delayed */, + false /* use_ms_signed_binary */, + false /* add_dll_permission */, + false /* add_directory_permission */); +} + +// This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS +// mitigation allows the loading of an unsigned DLL if intercept in place. +TEST(ProcessMitigationsTest, CheckWin10MsSignedWithIntercept_Success) { + if (base::win::GetVersion() < base::win::Version::WIN10_TH2) + return; + + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); + + // Expect success; Enable mitigation; Use non MS-signed binary. +#if defined(COMPONENT_BUILD) + // In a component build, add the directory to the allowed list. + TestWin10MsSigned(sandbox::SBOX_TEST_SUCCEEDED /* expected */, + true /* enable_mitigation */, + false /* delayed */, + false /* use_ms_signed_binary */, + true /* add_dll_permission */, + true /* add_directory_permission */); +#else + TestWin10MsSigned(sandbox::SBOX_TEST_SUCCEEDED /* expected */, + true /* enable_mitigation */, + false /* delayed */, + false /* use_ms_signed_binary */, + true /* add_dll_permission */, + false /* add_directory_permission */); +#endif // defined(COMPONENT_BUILD) +} - // Expect failure; Enable mitigation; Use non MS-signed binary. - TestWin10MsSigned(false, true, false); +// This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS +// mitigation pre-load prevents the loading of an unsigned DLL. +TEST(ProcessMitigationsTest, CheckWin10MsSigned_FailurePreSpawn) { + if (base::win::GetVersion() < base::win::Version::WIN10_TH2) + return; - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); + +#if defined(COMPONENT_BUILD) + // In a component build, the executable will fail to start-up because + // imports e.g. base.dll cannot be resolved. + int expected = STATUS_INVALID_IMAGE_HASH; +#else + // In a non-component build, the process will start, but the unsigned + // DLL will fail to load inside the test itself. + int expected = sandbox::SBOX_TEST_FAILED; +#endif + + TestWin10MsSigned(expected /* expected */, + true /* enable_mitigation */, + false /* delayed */, + false /* use_ms_signed_binary */, + false /* add_dll_permission */, + false /* add_directory_permission */); } // This test validates that we can load a signed Microsoft DLL if the @@ -781,17 +885,14 @@ TEST(ProcessMitigationsTest, CheckWin10MsSigned_MsBaseline) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); - // Expect success; Do not enable mitigation; Use MS-signed binary. - TestWin10MsSigned(true, false, true); - - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10MsSigned(sandbox::SBOX_TEST_SUCCEEDED /* expected */, + false /* enable_mitigation */, + false /* delayed */, + true /* use_ms_signed_binary */, + false /* add_dll_permission */, + false /* add_directory_permission */); } // This test validates that setting the MITIGATION_FORCE_MS_SIGNED_BINS @@ -800,17 +901,14 @@ TEST(ProcessMitigationsTest, CheckWin10MsSigned_MsSuccess) { if (base::win::GetVersion() < base::win::Version::WIN10_TH2) return; - HANDLE mutex = - ::CreateMutexW(nullptr, false, hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(mutex); - EXPECT_EQ(WAIT_OBJECT_0, - ::WaitForSingleObject(mutex, SboxTestEventTimeout())); - - // Expect success; Enable mitigation; Use MS-signed binary. - TestWin10MsSigned(true, true, true); + ScopedTestMutex mutex(hooking_dll::g_hooking_dll_mutex); - EXPECT_TRUE(::ReleaseMutex(mutex)); - EXPECT_TRUE(::CloseHandle(mutex)); + TestWin10MsSigned(sandbox::SBOX_TEST_SUCCEEDED /* expected */, + true /* enable_mitigation */, + true /* delayed */, + true /* use_ms_signed_binary */, + false /* add_dll_permission */, + false /* add_directory_permission */); } //------------------------------------------------------------------------------ diff --git a/chromium/sandbox/win/src/process_mitigations_win32k_dispatcher.cc b/chromium/sandbox/win/src/process_mitigations_win32k_dispatcher.cc index d37f54571fe..485c36d0ed1 100644 --- a/chromium/sandbox/win/src/process_mitigations_win32k_dispatcher.cc +++ b/chromium/sandbox/win/src/process_mitigations_win32k_dispatcher.cc @@ -6,7 +6,8 @@ #include <algorithm> -#include "base/memory/shared_memory.h" +#include "base/memory/platform_shared_memory_region.h" +#include "base/memory/unsafe_shared_memory_region.h" #include "base/strings/string16.h" #include "base/unguessable_token.h" #include "base/win/windows_version.h" @@ -20,18 +21,27 @@ namespace sandbox { namespace { -base::SharedMemoryHandle GetSharedMemoryHandle(const ClientInfo& client_info, - HANDLE handle, - size_t size) { - HANDLE result_handle = nullptr; +base::UnsafeSharedMemoryRegion GetSharedMemoryRegion( + const ClientInfo& client_info, + HANDLE handle, + size_t size) { + HANDLE dup_handle = nullptr; intptr_t handle_int = reinterpret_cast<intptr_t>(handle); if (handle_int <= 0 || !::DuplicateHandle(client_info.process, handle, ::GetCurrentProcess(), - &result_handle, 0, false, DUPLICATE_SAME_ACCESS)) { - result_handle = nullptr; - } - return base::SharedMemoryHandle(result_handle, size, - base::UnguessableToken::Create()); + &dup_handle, 0, false, DUPLICATE_SAME_ACCESS)) { + return {}; + } + // The raw handle returned from ::DuplicateHandle() must be wrapped in a + // base::PlatformSharedMemoryRegion. + base::subtle::PlatformSharedMemoryRegion platform_region = + base::subtle::PlatformSharedMemoryRegion::Take( + base::win::ScopedHandle(dup_handle), + base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe, size, + base::UnguessableToken::Create()); + // |platform_region| can now be wrapped in a base::UnsafeSharedMemoryRegion. + return base::UnsafeSharedMemoryRegion::Deserialize( + std::move(platform_region)); } } // namespace @@ -413,22 +423,22 @@ bool ProcessMitigationsWin32KDispatcher::GetCertificate( ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } - base::SharedMemoryHandle handle = GetSharedMemoryHandle( + base::UnsafeSharedMemoryRegion region = GetSharedMemoryRegion( *ipc->client_info, shared_buffer_handle, shared_buffer_size); - if (!handle.IsValid()) { + if (!region.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } - base::SharedMemory cert_data(handle, false); - if (!cert_data.Map(shared_buffer_size)) { + base::WritableSharedMemoryMapping cert_data = region.Map(); + if (!cert_data.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } NTSTATUS status = STATUS_INVALID_PARAMETER; if (device_name->size() > 0) { status = ProcessMitigationsWin32KLockdownPolicy::GetCertificateAction( - *ipc->client_info, *device_name, static_cast<BYTE*>(cert_data.memory()), - shared_buffer_size); + *ipc->client_info, *device_name, + cert_data.GetMemoryAsSpan<BYTE>().data(), shared_buffer_size); } else { scoped_refptr<ProtectedVideoOutput> output = GetProtectedVideoOutput(protected_output, false); @@ -436,7 +446,7 @@ bool ProcessMitigationsWin32KDispatcher::GetCertificate( status = ProcessMitigationsWin32KLockdownPolicy::GetCertificateByHandleAction( *ipc->client_info, output.get()->handle(), - static_cast<BYTE*>(cert_data.memory()), shared_buffer_size); + cert_data.GetMemoryAsSpan<BYTE>().data(), shared_buffer_size); } } ipc->return_info.nt_status = status; @@ -517,15 +527,15 @@ bool ProcessMitigationsWin32KDispatcher::ConfigureOPMProtectedOutput( ipc->return_info.nt_status = STATUS_INVALID_HANDLE; return true; }; - base::SharedMemoryHandle handle = - GetSharedMemoryHandle(*ipc->client_info, shared_buffer_handle, + base::UnsafeSharedMemoryRegion region = + GetSharedMemoryRegion(*ipc->client_info, shared_buffer_handle, sizeof(DXGKMDT_OPM_CONFIGURE_PARAMETERS)); - if (!handle.IsValid()) { + if (!region.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } - base::SharedMemory buffer(handle, false); - if (!buffer.Map(sizeof(DXGKMDT_OPM_CONFIGURE_PARAMETERS))) { + base::WritableSharedMemoryMapping buffer = region.Map(); + if (!buffer.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } @@ -554,15 +564,15 @@ bool ProcessMitigationsWin32KDispatcher::GetOPMInformation( std::max(sizeof(DXGKMDT_OPM_GET_INFO_PARAMETERS), sizeof(DXGKMDT_OPM_REQUESTED_INFORMATION)); - base::SharedMemoryHandle handle = GetSharedMemoryHandle( + base::UnsafeSharedMemoryRegion region = GetSharedMemoryRegion( *ipc->client_info, shared_buffer_handle, shared_buffer_size); - if (!handle.IsValid()) { + if (!region.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } - base::SharedMemory buffer(handle, false); - if (!buffer.Map(shared_buffer_size)) { + base::WritableSharedMemoryMapping buffer = region.Map(); + if (!buffer.IsValid()) { ipc->return_info.nt_status = STATUS_ACCESS_DENIED; return true; } diff --git a/chromium/sandbox/win/src/sandbox_nt_types.h b/chromium/sandbox/win/src/sandbox_nt_types.h index a5a40ceb134..eff0f019d3d 100644 --- a/chromium/sandbox/win/src/sandbox_nt_types.h +++ b/chromium/sandbox/win/src/sandbox_nt_types.h @@ -10,28 +10,30 @@ namespace sandbox { struct NtExports { - NtAllocateVirtualMemoryFunction AllocateVirtualMemory; - NtCloseFunction Close; - NtDuplicateObjectFunction DuplicateObject; - NtFreeVirtualMemoryFunction FreeVirtualMemory; - NtMapViewOfSectionFunction MapViewOfSection; - NtProtectVirtualMemoryFunction ProtectVirtualMemory; - NtQueryInformationProcessFunction QueryInformationProcess; - NtQueryObjectFunction QueryObject; - NtQuerySectionFunction QuerySection; - NtQueryVirtualMemoryFunction QueryVirtualMemory; - NtUnmapViewOfSectionFunction UnmapViewOfSection; - RtlAllocateHeapFunction RtlAllocateHeap; - RtlAnsiStringToUnicodeStringFunction RtlAnsiStringToUnicodeString; - RtlCompareUnicodeStringFunction RtlCompareUnicodeString; - RtlCreateHeapFunction RtlCreateHeap; - RtlCreateUserThreadFunction RtlCreateUserThread; - RtlDestroyHeapFunction RtlDestroyHeap; - RtlFreeHeapFunction RtlFreeHeap; - _strnicmpFunction _strnicmp; - strlenFunction strlen; - wcslenFunction wcslen; - memcpyFunction memcpy; + NtAllocateVirtualMemoryFunction AllocateVirtualMemory; + NtCloseFunction Close; + NtDuplicateObjectFunction DuplicateObject; + NtFreeVirtualMemoryFunction FreeVirtualMemory; + NtMapViewOfSectionFunction MapViewOfSection; + NtProtectVirtualMemoryFunction ProtectVirtualMemory; + NtQueryInformationProcessFunction QueryInformationProcess; + NtQueryObjectFunction QueryObject; + NtQuerySectionFunction QuerySection; + NtQueryVirtualMemoryFunction QueryVirtualMemory; + NtUnmapViewOfSectionFunction UnmapViewOfSection; + NtSignalAndWaitForSingleObjectFunction SignalAndWaitForSingleObject; + NtWaitForSingleObjectFunction WaitForSingleObject; + RtlAllocateHeapFunction RtlAllocateHeap; + RtlAnsiStringToUnicodeStringFunction RtlAnsiStringToUnicodeString; + RtlCompareUnicodeStringFunction RtlCompareUnicodeString; + RtlCreateHeapFunction RtlCreateHeap; + RtlCreateUserThreadFunction RtlCreateUserThread; + RtlDestroyHeapFunction RtlDestroyHeap; + RtlFreeHeapFunction RtlFreeHeap; + _strnicmpFunction _strnicmp; + strlenFunction strlen; + wcslenFunction wcslen; + memcpyFunction memcpy; }; // This is the value used for the ntdll level allocator. diff --git a/chromium/sandbox/win/src/sandbox_nt_util.cc b/chromium/sandbox/win/src/sandbox_nt_util.cc index f71177fd7a9..bace3102cbc 100644 --- a/chromium/sandbox/win/src/sandbox_nt_util.cc +++ b/chromium/sandbox/win/src/sandbox_nt_util.cc @@ -646,6 +646,40 @@ bool IsSupportedRenameCall(FILE_RENAME_INFORMATION* file_info, return true; } +bool NtGetPathFromHandle(HANDLE handle, + std::unique_ptr<wchar_t, NtAllocDeleter>* path) { + OBJECT_NAME_INFORMATION initial_buffer; + OBJECT_NAME_INFORMATION* name; + ULONG size = 0; + // Query the name information a first time to get the size of the name. + NTSTATUS status = g_nt.QueryObject(handle, ObjectNameInformation, + &initial_buffer, size, &size); + + if (!NT_SUCCESS(status) && status != STATUS_INFO_LENGTH_MISMATCH) + return false; + + std::unique_ptr<BYTE[], NtAllocDeleter> name_ptr; + if (!size) + return false; + name_ptr.reset(new (NT_ALLOC) BYTE[size]); + name = reinterpret_cast<OBJECT_NAME_INFORMATION*>(name_ptr.get()); + + // Query the name information a second time to get the name of the + // object referenced by the handle. + status = g_nt.QueryObject(handle, ObjectNameInformation, name, size, &size); + + if (STATUS_SUCCESS != status) + return false; + size_t num_path_wchars = (name->ObjectName.Length / sizeof(wchar_t)) + 1; + path->reset(new (NT_ALLOC) wchar_t[num_path_wchars]); + status = + CopyData(path->get(), name->ObjectName.Buffer, name->ObjectName.Length); + path->get()[num_path_wchars - 1] = L'\0'; + if (STATUS_SUCCESS != status) + return false; + return true; +} + } // namespace sandbox void* operator new(size_t size, sandbox::AllocationType type, void* near_to) { diff --git a/chromium/sandbox/win/src/sandbox_nt_util.h b/chromium/sandbox/win/src/sandbox_nt_util.h index 08880d19299..e32e4ccb569 100644 --- a/chromium/sandbox/win/src/sandbox_nt_util.h +++ b/chromium/sandbox/win/src/sandbox_nt_util.h @@ -179,6 +179,10 @@ bool IsValidImageSection(HANDLE section, // Converts an ansi string to an UNICODE_STRING. UNICODE_STRING* AnsiToUnicode(const char* string); +// Resolves a handle to an nt path. Returns true if the handle can be resolved. +bool NtGetPathFromHandle(HANDLE handle, + std::unique_ptr<wchar_t, NtAllocDeleter>* path); + // Provides a simple way to temporarily change the protection of a memory page. class AutoProtectMemory { public: diff --git a/chromium/sandbox/win/src/sandbox_nt_util_unittest.cc b/chromium/sandbox/win/src/sandbox_nt_util_unittest.cc index 4471f6cdc3e..a98392fe55d 100644 --- a/chromium/sandbox/win/src/sandbox_nt_util_unittest.cc +++ b/chromium/sandbox/win/src/sandbox_nt_util_unittest.cc @@ -9,9 +9,13 @@ #include <memory> #include <vector> +#include "base/files/file.h" +#include "base/path_service.h" +#include "base/strings/string_util.h" #include "base/win/scoped_handle.h" #include "base/win/scoped_process_information.h" #include "sandbox/win/src/policy_broker.h" +#include "sandbox/win/src/win_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace sandbox { @@ -237,5 +241,26 @@ TEST(SandboxNtUtil, ValidParameter) { EXPECT_TRUE(verify_buffer()); } +TEST(SandboxNtUtil, NtGetPathFromHandle) { + InitGlobalNt(); + + base::FilePath exe; + ASSERT_TRUE(base::PathService::Get(base::FILE_EXE, &exe)); + base::File exe_file(exe, base::File::FLAG_OPEN); + ASSERT_TRUE(exe_file.IsValid()); + std::unique_ptr<wchar_t, NtAllocDeleter> path; + EXPECT_TRUE(NtGetPathFromHandle(exe_file.GetPlatformFile(), &path)); + + // Basic sanity test, the functionality of NtGetPathFromHandle to return + // the correct value is already tested from win_utils_unittest.cc. + EXPECT_TRUE(base::EndsWith(path.get(), exe.BaseName().value(), + base::CompareCase::INSENSITIVE_ASCII)); + + // Compare to GetNtPathFromWin32Path for extra check. + base::string16 nt_path; + EXPECT_TRUE(GetNtPathFromWin32Path(exe.value(), &nt_path)); + EXPECT_STREQ(path.get(), nt_path.c_str()); +} + } // namespace } // namespace sandbox diff --git a/chromium/sandbox/win/src/sandbox_policy.h b/chromium/sandbox/win/src/sandbox_policy.h index 29ba0b7420b..205e9bafbf7 100644 --- a/chromium/sandbox/win/src/sandbox_policy.h +++ b/chromium/sandbox/win/src/sandbox_policy.h @@ -20,16 +20,17 @@ class AppContainerProfile; class TargetPolicy { public: // Windows subsystems that can have specific rules. - // Note: The process subsystem(SUBSY_PROCESS) does not evaluate the request + // Note: The process subsystem(SUBSYS_PROCESS) does not evaluate the request // exactly like the CreateProcess API does. See the comment at the top of // process_thread_dispatcher.cc for more details. enum SubSystem { - SUBSYS_FILES, // Creation and opening of files and pipes. - SUBSYS_NAMED_PIPES, // Creation of named pipes. - SUBSYS_PROCESS, // Creation of child processes. - SUBSYS_REGISTRY, // Creation and opening of registry keys. - SUBSYS_SYNC, // Creation of named sync objects. - SUBSYS_WIN32K_LOCKDOWN // Win32K Lockdown related policy. + SUBSYS_FILES, // Creation and opening of files and pipes. + SUBSYS_NAMED_PIPES, // Creation of named pipes. + SUBSYS_PROCESS, // Creation of child processes. + SUBSYS_REGISTRY, // Creation and opening of registry keys. + SUBSYS_SYNC, // Creation of named sync objects. + SUBSYS_WIN32K_LOCKDOWN, // Win32K Lockdown related policy. + SUBSYS_SIGNED_BINARY // Signed binary policy. }; // Allowable semantics when a rule is matched. @@ -56,9 +57,10 @@ class TargetPolicy { FAKE_USER_GDI_INIT, // Fakes user32 and gdi32 initialization. This can // be used to allow the DLLs to load and initialize // even if the process cannot access that subsystem. - IMPLEMENT_OPM_APIS // Implements FAKE_USER_GDI_INIT and also exposes + IMPLEMENT_OPM_APIS, // Implements FAKE_USER_GDI_INIT and also exposes // IPC calls to handle Output Protection Manager // APIs. + SIGNED_ALLOW_LOAD // Allows loading the module when CIG is enabled. }; // Increments the reference count of this object. The reference count must diff --git a/chromium/sandbox/win/src/sandbox_policy_base.cc b/chromium/sandbox/win/src/sandbox_policy_base.cc index 3287ac4d193..a47151d6562 100644 --- a/chromium/sandbox/win/src/sandbox_policy_base.cc +++ b/chromium/sandbox/win/src/sandbox_policy_base.cc @@ -30,6 +30,7 @@ #include "sandbox/win/src/sandbox_policy.h" #include "sandbox/win/src/sandbox_utils.h" #include "sandbox/win/src/security_capabilities.h" +#include "sandbox/win/src/signed_policy.h" #include "sandbox/win/src/sync_policy.h" #include "sandbox/win/src/target_process.h" #include "sandbox/win/src/top_level_dispatcher.h" @@ -355,7 +356,7 @@ ResultCode PolicyBase::AddRule(SubSystem subsystem, } ResultCode PolicyBase::AddDllToUnload(const wchar_t* dll_name) { - blacklisted_dlls_.push_back(dll_name); + blocklisted_dlls_.push_back(dll_name); return SBOX_ALL_OK; } @@ -659,7 +660,7 @@ ResultCode PolicyBase::SetupAllInterceptions(TargetProcess* target) { } } - for (const base::string16& dll : blacklisted_dlls_) + for (const base::string16& dll : blocklisted_dlls_) manager.AddToUnloadModules(dll.c_str()); if (!SetupBasicInterceptions(&manager, is_csrss_connected_)) @@ -745,6 +746,17 @@ ResultCode PolicyBase::AddRuleInternal(SubSystem subsystem, } break; } + case SUBSYS_SIGNED_BINARY: { + // These rules only need to be added if the + // MITIGATION_FORCE_MS_SIGNED_BINS pre-startup mitigation is set. + if (mitigations_ & MITIGATION_FORCE_MS_SIGNED_BINS) { + if (!SignedPolicy::GenerateRules(pattern, semantics, policy_maker_)) { + NOTREACHED(); + return SBOX_ERROR_BAD_PARAMS; + } + } + break; + } default: { return SBOX_ERROR_UNSUPPORTED; } } diff --git a/chromium/sandbox/win/src/sandbox_policy_base.h b/chromium/sandbox/win/src/sandbox_policy_base.h index cb733981f48..631f974df70 100644 --- a/chromium/sandbox/win/src/sandbox_policy_base.h +++ b/chromium/sandbox/win/src/sandbox_policy_base.h @@ -156,7 +156,7 @@ class PolicyBase final : public TargetPolicy { // Memory structure that stores the low level policy. PolicyGlobal* policy_; // The list of dlls to unload in the target process. - std::vector<base::string16> blacklisted_dlls_; + std::vector<base::string16> blocklisted_dlls_; // This is a map of handle-types to names that we need to close in the // target process. A null set means we need to close all handles of the // given type. diff --git a/chromium/sandbox/win/src/sharedmem_ipc_client.cc b/chromium/sandbox/win/src/sharedmem_ipc_client.cc index 3163c67972a..2f849be31a9 100644 --- a/chromium/sandbox/win/src/sharedmem_ipc_client.cc +++ b/chromium/sandbox/win/src/sharedmem_ipc_client.cc @@ -11,9 +11,50 @@ #include "sandbox/win/src/crosscall_client.h" #include "sandbox/win/src/crosscall_params.h" #include "sandbox/win/src/sandbox.h" +#include "sandbox/win/src/sandbox_nt_types.h" +#include "sandbox/win/src/sandbox_nt_util.h" namespace sandbox { +SANDBOX_INTERCEPT NtExports g_nt; + +namespace { + +DWORD SignalObjectAndWaitWrapper(HANDLE object_to_signal, + HANDLE object_to_wait_on, + DWORD millis, + BOOL alertable) { + // Not running in a sandboxed process so can call directly. + if (!g_nt.SignalAndWaitForSingleObject) + return SignalObjectAndWait(object_to_signal, object_to_wait_on, millis, + alertable); + // Don't support alertable. + CHECK_NT(!alertable); + LARGE_INTEGER timeout; + timeout.QuadPart = millis * -10000LL; + NTSTATUS status = g_nt.SignalAndWaitForSingleObject( + object_to_signal, object_to_wait_on, alertable, + millis == INFINITE ? nullptr : &timeout); + if (!NT_SUCCESS(status)) + return WAIT_FAILED; + return status; +} + +DWORD WaitForSingleObjectWrapper(HANDLE handle, DWORD millis) { + // Not running in a sandboxed process so can call directly. + if (!g_nt.WaitForSingleObject) + return WaitForSingleObject(handle, millis); + LARGE_INTEGER timeout; + timeout.QuadPart = millis * -10000LL; + NTSTATUS status = g_nt.WaitForSingleObject( + handle, FALSE, millis == INFINITE ? nullptr : &timeout); + if (!NT_SUCCESS(status)) + return WAIT_FAILED; + return status; +} + +} // namespace + // Get the base of the data buffer of the channel; this is where the input // parameters get serialized. Since they get serialized directly into the // channel we avoid one copy. @@ -68,18 +109,19 @@ ResultCode SharedMemIPCClient::DoCall(CrossCallParams* params, // While the atomic signaling and waiting is not a requirement, it // is nice because we save a trip to kernel. - DWORD wait = - ::SignalObjectAndWait(channel[num].ping_event, channel[num].pong_event, - kIPCWaitTimeOut1, false); + DWORD wait = SignalObjectAndWaitWrapper(channel[num].ping_event, + channel[num].pong_event, + kIPCWaitTimeOut1, false); if (WAIT_TIMEOUT == wait) { // The server is taking too long. Enter a loop were we check if the // server_alive mutex has been abandoned which would signal a server crash // or else we keep waiting for a response. while (true) { - wait = ::WaitForSingleObject(control_->server_alive, 0); + wait = WaitForSingleObjectWrapper(control_->server_alive, 0); if (WAIT_TIMEOUT == wait) { // Server seems still alive. We already signaled so here we just wait. - wait = ::WaitForSingleObject(channel[num].pong_event, kIPCWaitTimeOut1); + wait = WaitForSingleObjectWrapper(channel[num].pong_event, + kIPCWaitTimeOut1); if (WAIT_OBJECT_0 == wait) { // The server took a long time but responded. break; @@ -131,7 +173,7 @@ size_t SharedMemIPCClient::LockFreeChannel(bool* severe_failure) { } // We did not find any available channel, maybe the server is dead. DWORD wait = - ::WaitForSingleObject(control_->server_alive, kIPCWaitTimeOut2); + WaitForSingleObjectWrapper(control_->server_alive, kIPCWaitTimeOut2); if (WAIT_TIMEOUT != wait) { // The server is dead and we outlive it enough to get in trouble. *severe_failure = true; diff --git a/chromium/sandbox/win/src/signed_dispatcher.cc b/chromium/sandbox/win/src/signed_dispatcher.cc new file mode 100644 index 00000000000..325a81e4159 --- /dev/null +++ b/chromium/sandbox/win/src/signed_dispatcher.cc @@ -0,0 +1,66 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sandbox/win/src/signed_dispatcher.h" + +#include <stdint.h> + +#include "base/strings/string16.h" +#include "base/strings/string_util.h" +#include "base/win/scoped_handle.h" +#include "sandbox/win/src/crosscall_client.h" +#include "sandbox/win/src/interception.h" +#include "sandbox/win/src/interceptors.h" +#include "sandbox/win/src/ipc_tags.h" +#include "sandbox/win/src/policy_params.h" +#include "sandbox/win/src/sandbox.h" +#include "sandbox/win/src/signed_interception.h" +#include "sandbox/win/src/signed_policy.h" + +namespace sandbox { + +SignedDispatcher::SignedDispatcher(PolicyBase* policy_base) + : policy_base_(policy_base) { + static const IPCCall create_params = { + {IPC_NTCREATESECTION_TAG, {VOIDPTR_TYPE}}, + reinterpret_cast<CallbackGeneric>(&SignedDispatcher::CreateSection)}; + + ipc_calls_.push_back(create_params); +} + +bool SignedDispatcher::SetupService(InterceptionManager* manager, int service) { + if (service == IPC_NTCREATESECTION_TAG) + return INTERCEPT_NT(manager, NtCreateSection, CREATE_SECTION_ID, 32); + return false; +} + +bool SignedDispatcher::CreateSection(IPCInfo* ipc, HANDLE file_handle) { + // Duplicate input handle from target to broker. + HANDLE local_file_handle = nullptr; + if (!::DuplicateHandle((*ipc->client_info).process, file_handle, + ::GetCurrentProcess(), &local_file_handle, + FILE_MAP_EXECUTE, false, 0)) { + return false; + } + + base::win::ScopedHandle local_handle(local_file_handle); + base::string16 path; + if (!GetPathFromHandle(local_handle.Get(), &path)) + return false; + const wchar_t* module_name = path.c_str(); + CountedParameterSet<NameBased> params; + params[NameBased::NAME] = ParamPickerMake(module_name); + + EvalResult result = + policy_base_->EvalPolicy(IPC_NTCREATESECTION_TAG, params.GetBase()); + + // Return operation status on the IPC. + HANDLE section_handle = nullptr; + ipc->return_info.nt_status = SignedPolicy::CreateSectionAction( + result, *ipc->client_info, local_handle, §ion_handle); + ipc->return_info.handle = section_handle; + return true; +} + +} // namespace sandbox diff --git a/chromium/sandbox/win/src/signed_dispatcher.h b/chromium/sandbox/win/src/signed_dispatcher.h new file mode 100644 index 00000000000..7abff807ccd --- /dev/null +++ b/chromium/sandbox/win/src/signed_dispatcher.h @@ -0,0 +1,36 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SANDBOX_WIN_SRC_SIGNED_DISPATCHER_H_ +#define SANDBOX_WIN_SRC_SIGNED_DISPATCHER_H_ + +#include <stdint.h> + +#include "base/macros.h" +#include "sandbox/win/src/crosscall_server.h" +#include "sandbox/win/src/interception.h" +#include "sandbox/win/src/sandbox_policy_base.h" + +namespace sandbox { + +// This class handles signed-binary related IPC calls. +class SignedDispatcher : public Dispatcher { + public: + explicit SignedDispatcher(PolicyBase* policy_base); + ~SignedDispatcher() override {} + + // Dispatcher interface. + bool SetupService(InterceptionManager* manager, int service) override; + + private: + // Processes IPC requests coming from calls to CreateSection in the target. + bool CreateSection(IPCInfo* ipc, HANDLE file_handle); + + PolicyBase* policy_base_; + DISALLOW_COPY_AND_ASSIGN(SignedDispatcher); +}; + +} // namespace sandbox + +#endif // SANDBOX_WIN_SRC_SIGNED_DISPATCHER_H_ diff --git a/chromium/sandbox/win/src/signed_interception.cc b/chromium/sandbox/win/src/signed_interception.cc new file mode 100644 index 00000000000..c0715215487 --- /dev/null +++ b/chromium/sandbox/win/src/signed_interception.cc @@ -0,0 +1,90 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sandbox/win/src/signed_interception.h" + +#include <stdint.h> + +#include "sandbox/win/src/crosscall_client.h" +#include "sandbox/win/src/ipc_tags.h" +#include "sandbox/win/src/policy_params.h" +#include "sandbox/win/src/policy_target.h" +#include "sandbox/win/src/sandbox_factory.h" +#include "sandbox/win/src/sandbox_nt_util.h" +#include "sandbox/win/src/sharedmem_ipc_client.h" +#include "sandbox/win/src/target_services.h" + +namespace sandbox { + +NTSTATUS WINAPI +TargetNtCreateSection(NtCreateSectionFunction orig_CreateSection, + PHANDLE section_handle, + ACCESS_MASK desired_access, + POBJECT_ATTRIBUTES object_attributes, + PLARGE_INTEGER maximum_size, + ULONG section_page_protection, + ULONG allocation_attributes, + HANDLE file_handle) { + NTSTATUS status = orig_CreateSection( + section_handle, desired_access, object_attributes, maximum_size, + section_page_protection, allocation_attributes, file_handle); + + // Only intercept calls that match a particular signature. + if (status != STATUS_INVALID_IMAGE_HASH) + return status; + if (desired_access != (SECTION_QUERY | SECTION_MAP_WRITE | SECTION_MAP_READ | + SECTION_MAP_EXECUTE)) + return status; + if (object_attributes) + return status; + if (maximum_size) + return status; + if (section_page_protection != PAGE_EXECUTE) + return status; + if (allocation_attributes != SEC_IMAGE) + return status; + + do { + if (!ValidParameter(section_handle, sizeof(HANDLE), WRITE)) + break; + + void* memory = GetGlobalIPCMemory(); + if (!memory) + break; + std::unique_ptr<wchar_t, NtAllocDeleter> path; + if (!NtGetPathFromHandle(file_handle, &path)) + break; + const wchar_t* const_name = path.get(); + + CountedParameterSet<NameBased> params; + params[NameBased::NAME] = ParamPickerMake(const_name); + + if (!QueryBroker(IPC_NTCREATESECTION_TAG, params.GetBase())) + break; + + CrossCallReturn answer = {0}; + answer.nt_status = status; + SharedMemIPCClient ipc(memory); + ResultCode code = + CrossCall(ipc, IPC_NTCREATESECTION_TAG, file_handle, &answer); + + if (code != SBOX_ALL_OK) + break; + + status = answer.nt_status; + + if (!NT_SUCCESS(answer.nt_status)) + break; + + __try { + *section_handle = answer.handle; + } __except (EXCEPTION_EXECUTE_HANDLER) { + break; + } + } while (false); + + return status; +} + +} // namespace sandbox diff --git a/chromium/sandbox/win/src/signed_interception.h b/chromium/sandbox/win/src/signed_interception.h new file mode 100644 index 00000000000..a50ec38222b --- /dev/null +++ b/chromium/sandbox/win/src/signed_interception.h @@ -0,0 +1,30 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SANDBOX_WIN_SRC_SIGNED_INTERCEPTION_H_ +#define SANDBOX_WIN_SRC_SIGNED_INTERCEPTION_H_ + +#include "sandbox/win/src/nt_internals.h" +#include "sandbox/win/src/sandbox_types.h" + +namespace sandbox { + +extern "C" { + +// Interceptor for NtCreateSection +SANDBOX_INTERCEPT NTSTATUS WINAPI +TargetNtCreateSection(NtCreateSectionFunction orig_CreateSection, + PHANDLE section_handle, + ACCESS_MASK desired_access, + POBJECT_ATTRIBUTES object_attributes, + PLARGE_INTEGER maximum_size, + ULONG section_page_protection, + ULONG allocation_attributes, + HANDLE file_handle); + +} // extern "C" + +} // namespace sandbox + +#endif // SANDBOX_WIN_SRC_SIGNED_INTERCEPTION_H_ diff --git a/chromium/sandbox/win/src/signed_policy.cc b/chromium/sandbox/win/src/signed_policy.cc new file mode 100644 index 00000000000..6b7b11f656a --- /dev/null +++ b/chromium/sandbox/win/src/signed_policy.cc @@ -0,0 +1,78 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "sandbox/win/src/signed_policy.h" + +#include <stdint.h> + +#include <string> + +#include "sandbox/win/src/ipc_tags.h" +#include "sandbox/win/src/policy_engine_opcodes.h" +#include "sandbox/win/src/policy_params.h" +#include "sandbox/win/src/sandbox_policy.h" +#include "sandbox/win/src/win_utils.h" + +namespace sandbox { + +bool SignedPolicy::GenerateRules(const wchar_t* name, + TargetPolicy::Semantics semantics, + LowLevelPolicy* policy) { + // Only support one semantic. + if (TargetPolicy::SIGNED_ALLOW_LOAD != semantics) { + return false; + } + + base::FilePath file_path(name); + base::string16 nt_path_name; + if (!GetNtPathFromWin32Path(file_path.DirName().value().c_str(), + &nt_path_name)) + return false; + base::FilePath nt_path(nt_path_name); + base::string16 nt_filename = nt_path.Append(file_path.BaseName()).value(); + // Create a rule to ASK_BROKER if name matches. + PolicyRule signed_policy(ASK_BROKER); + if (!signed_policy.AddStringMatch(IF, NameBased::NAME, nt_filename.c_str(), + CASE_INSENSITIVE)) { + return false; + } + if (!policy->AddRule(IPC_NTCREATESECTION_TAG, &signed_policy)) { + return false; + } + + return true; +} + +NTSTATUS SignedPolicy::CreateSectionAction( + EvalResult eval_result, + const ClientInfo& client_info, + const base::win::ScopedHandle& local_file_handle, + HANDLE* target_section_handle) { + NtCreateSectionFunction NtCreateSection = nullptr; + ResolveNTFunctionPtr("NtCreateSection", &NtCreateSection); + + // The only action supported is ASK_BROKER which means create the requested + // section as specified. + if (ASK_BROKER != eval_result) + return false; + + HANDLE local_section_handle = nullptr; + NTSTATUS status = NtCreateSection(&local_section_handle, + SECTION_QUERY | SECTION_MAP_WRITE | + SECTION_MAP_READ | SECTION_MAP_EXECUTE, + nullptr, 0, PAGE_EXECUTE, SEC_IMAGE, + local_file_handle.Get()); + if (!local_section_handle) + return status; + + // Duplicate section handle back to the target. + if (!::DuplicateHandle(::GetCurrentProcess(), local_section_handle, + client_info.process, target_section_handle, 0, false, + DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { + return STATUS_ACCESS_DENIED; + } + return status; +} + +} // namespace sandbox diff --git a/chromium/sandbox/win/src/signed_policy.h b/chromium/sandbox/win/src/signed_policy.h new file mode 100644 index 00000000000..d22af4d4dc4 --- /dev/null +++ b/chromium/sandbox/win/src/signed_policy.h @@ -0,0 +1,39 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SANDBOX_WIN_SRC_SIGNED_POLICY_H_ +#define SANDBOX_WIN_SRC_SIGNED_POLICY_H_ + +#include <stdint.h> + +#include "base/win/scoped_handle.h" +#include "sandbox/win/src/crosscall_server.h" +#include "sandbox/win/src/policy_engine_opcodes.h" +#include "sandbox/win/src/policy_low_level.h" +#include "sandbox/win/src/sandbox_policy.h" + +namespace sandbox { + +// This class centralizes most of the knowledge related to signed policy +class SignedPolicy { + public: + // Creates the required low-level policy rules to evaluate a high-level + // policy rule. + static bool GenerateRules(const wchar_t* name, + TargetPolicy::Semantics semantics, + LowLevelPolicy* policy); + + // Performs the desired policy action on a request. + // client_info is the target process that is making the request and + // eval_result is the desired policy action to accomplish. + static NTSTATUS CreateSectionAction( + EvalResult eval_result, + const ClientInfo& client_info, + const base::win::ScopedHandle& local_file_handle, + HANDLE* section_handle); +}; + +} // namespace sandbox + +#endif // SANDBOX_WIN_SRC_SIGNED_POLICY_H_ diff --git a/chromium/sandbox/win/src/target_interceptions.cc b/chromium/sandbox/win/src/target_interceptions.cc index 7ea741c6c18..2dc2fd1456d 100644 --- a/chromium/sandbox/win/src/target_interceptions.cc +++ b/chromium/sandbox/win/src/target_interceptions.cc @@ -7,7 +7,6 @@ #include "sandbox/win/src/interception_agent.h" #include "sandbox/win/src/sandbox_factory.h" #include "sandbox/win/src/sandbox_nt_util.h" -#include "sandbox/win/src/target_services.h" namespace sandbox { @@ -68,7 +67,6 @@ TargetNtMapViewOfSection(NtMapViewOfSectionFunction orig_MapViewOfSection, if (ansi_module_name && (g_nt._strnicmp(ansi_module_name, KERNEL32_DLL_NAME, sizeof(KERNEL32_DLL_NAME)) == 0)) { - SandboxFactory::GetTargetServices()->GetState()->SetKernel32Loaded(); s_state = kAfterKernel32; } } __except (EXCEPTION_EXECUTE_HANDLER) { diff --git a/chromium/sandbox/win/src/target_process.h b/chromium/sandbox/win/src/target_process.h index 72421660e87..f55fa466a11 100644 --- a/chromium/sandbox/win/src/target_process.h +++ b/chromium/sandbox/win/src/target_process.h @@ -98,7 +98,7 @@ class TargetProcess { // Returns the handle to the main thread. HANDLE MainThread() const { return sandbox_process_info_.thread_handle(); } - // Transfers a 32-bit variable between the broker and the target. + // Transfers variable at |address| of |size| bytes from broker to target. ResultCode TransferVariable(const char* name, void* address, size_t size); private: diff --git a/chromium/sandbox/win/src/target_services.cc b/chromium/sandbox/win/src/target_services.cc index c94678312b7..f35dc39daef 100644 --- a/chromium/sandbox/win/src/target_services.cc +++ b/chromium/sandbox/win/src/target_services.cc @@ -215,37 +215,29 @@ bool TargetServicesBase::TestIPCPing(int version) { return true; } -ProcessState::ProcessState() : process_state_(0), csrss_connected_(true) {} - -bool ProcessState::IsKernel32Loaded() const { - return process_state_ != 0; -} +ProcessState::ProcessState() + : process_state_(ProcessStateInternal::NONE), csrss_connected_(true) {} bool ProcessState::InitCalled() const { - return process_state_ > 1; + return process_state_ >= ProcessStateInternal::INIT_CALLED; } bool ProcessState::RevertedToSelf() const { - return process_state_ > 2; + return process_state_ >= ProcessStateInternal::REVERTED_TO_SELF; } bool ProcessState::IsCsrssConnected() const { return csrss_connected_; } -void ProcessState::SetKernel32Loaded() { - if (!process_state_) - process_state_ = 1; -} - void ProcessState::SetInitCalled() { - if (process_state_ < 2) - process_state_ = 2; + if (process_state_ < ProcessStateInternal::INIT_CALLED) + process_state_ = ProcessStateInternal::INIT_CALLED; } void ProcessState::SetRevertedToSelf() { - if (process_state_ < 3) - process_state_ = 3; + if (process_state_ < ProcessStateInternal::REVERTED_TO_SELF) + process_state_ = ProcessStateInternal::REVERTED_TO_SELF; } void ProcessState::SetCsrssConnected(bool csrss_connected) { diff --git a/chromium/sandbox/win/src/target_services.h b/chromium/sandbox/win/src/target_services.h index a33c92012f5..e50b7fb97c4 100644 --- a/chromium/sandbox/win/src/target_services.h +++ b/chromium/sandbox/win/src/target_services.h @@ -14,8 +14,6 @@ namespace sandbox { class ProcessState { public: ProcessState(); - // Returns true if kernel32.dll has been loaded. - bool IsKernel32Loaded() const; // Returns true if main has been called. bool InitCalled() const; // Returns true if LowerToken has been called. @@ -23,13 +21,14 @@ class ProcessState { // Returns true if Csrss is connected. bool IsCsrssConnected() const; // Set the current state. - void SetKernel32Loaded(); void SetInitCalled(); void SetRevertedToSelf(); void SetCsrssConnected(bool csrss_connected); private: - int process_state_; + enum class ProcessStateInternal { NONE = 0, INIT_CALLED, REVERTED_TO_SELF }; + + ProcessStateInternal process_state_; bool csrss_connected_; DISALLOW_COPY_AND_ASSIGN(ProcessState); }; diff --git a/chromium/sandbox/win/src/top_level_dispatcher.cc b/chromium/sandbox/win/src/top_level_dispatcher.cc index 9cbbdc20f96..f245802d64b 100644 --- a/chromium/sandbox/win/src/top_level_dispatcher.cc +++ b/chromium/sandbox/win/src/top_level_dispatcher.cc @@ -18,6 +18,7 @@ #include "sandbox/win/src/process_thread_dispatcher.h" #include "sandbox/win/src/registry_dispatcher.h" #include "sandbox/win/src/sandbox_policy_base.h" +#include "sandbox/win/src/signed_dispatcher.h" #include "sandbox/win/src/sync_dispatcher.h" namespace sandbox { @@ -76,6 +77,10 @@ TopLevelDispatcher::TopLevelDispatcher(PolicyBase* policy) : policy_(policy) { dispatcher; ipc_targets_[IPC_GDI_SETOPMSIGNINGKEYANDSEQUENCENUMBERS_TAG] = dispatcher; process_mitigations_win32k_dispatcher_.reset(dispatcher); + + dispatcher = new SignedDispatcher(policy_); + ipc_targets_[IPC_NTCREATESECTION_TAG] = dispatcher; + signed_dispatcher_.reset(dispatcher); } TopLevelDispatcher::~TopLevelDispatcher() {} diff --git a/chromium/sandbox/win/src/top_level_dispatcher.h b/chromium/sandbox/win/src/top_level_dispatcher.h index c1cf8f682a1..36f71aeb06b 100644 --- a/chromium/sandbox/win/src/top_level_dispatcher.h +++ b/chromium/sandbox/win/src/top_level_dispatcher.h @@ -42,6 +42,7 @@ class TopLevelDispatcher : public Dispatcher { std::unique_ptr<Dispatcher> registry_dispatcher_; std::unique_ptr<Dispatcher> handle_dispatcher_; std::unique_ptr<Dispatcher> process_mitigations_win32k_dispatcher_; + std::unique_ptr<Dispatcher> signed_dispatcher_; Dispatcher* ipc_targets_[IPC_LAST_TAG]; DISALLOW_COPY_AND_ASSIGN(TopLevelDispatcher); diff --git a/chromium/sandbox/win/src/win_utils.cc b/chromium/sandbox/win/src/win_utils.cc index 863b87495ca..2225c88f83e 100644 --- a/chromium/sandbox/win/src/win_utils.cc +++ b/chromium/sandbox/win/src/win_utils.cc @@ -474,6 +474,35 @@ bool WriteProtectedChildMemory(HANDLE child_process, return ok; } +bool CopyToChildMemory(HANDLE child, + const void* local_buffer, + size_t buffer_bytes, + void** remote_buffer) { + DCHECK(remote_buffer); + if (0 == buffer_bytes) { + *remote_buffer = nullptr; + return true; + } + + // Allocate memory in the target process without specifying the address + void* remote_data = ::VirtualAllocEx(child, nullptr, buffer_bytes, MEM_COMMIT, + PAGE_READWRITE); + if (!remote_data) + return false; + + SIZE_T bytes_written; + bool success = ::WriteProcessMemory(child, remote_data, local_buffer, + buffer_bytes, &bytes_written); + if (!success || bytes_written != buffer_bytes) { + ::VirtualFreeEx(child, remote_data, 0, MEM_RELEASE); + return false; + } + + *remote_buffer = remote_data; + + return true; +} + DWORD GetLastErrorFromNtStatus(NTSTATUS status) { RtlNtStatusToDosErrorFunction NtStatusToDosError = nullptr; ResolveNTFunctionPtr("RtlNtStatusToDosError", &NtStatusToDosError); diff --git a/chromium/sandbox/win/src/win_utils.h b/chromium/sandbox/win/src/win_utils.h index f69ddf70c03..fed7b4c3f35 100644 --- a/chromium/sandbox/win/src/win_utils.h +++ b/chromium/sandbox/win/src/win_utils.h @@ -119,6 +119,17 @@ bool WriteProtectedChildMemory(HANDLE child_process, const void* buffer, size_t length); +// Allocates |buffer_bytes| in child (PAGE_READWRITE) and copies data +// from |local_buffer| in this process into |child|. |remote_buffer| +// contains the address in the chile. If a zero byte copy is +// requested |true| is returned and no allocation or copying is +// attempted. Returns false if allocation or copying fails. If +// copying fails, the allocation will be reversed. +bool CopyToChildMemory(HANDLE child, + const void* local_buffer, + size_t buffer_bytes, + void** remote_buffer); + // Returns true if the provided path points to a pipe. bool IsPipe(const base::string16& path); diff --git a/chromium/sandbox/win/src/win_utils_unittest.cc b/chromium/sandbox/win/src/win_utils_unittest.cc index c814dbb3948..507ceafdd2d 100644 --- a/chromium/sandbox/win/src/win_utils_unittest.cc +++ b/chromium/sandbox/win/src/win_utils_unittest.cc @@ -20,6 +20,8 @@ #include "sandbox/win/tests/common/test_utils.h" #include "testing/gtest/include/gtest/gtest.h" +namespace sandbox { + namespace { class ScopedTerminateProcess { @@ -252,4 +254,6 @@ TEST(WinUtils, ConvertToLongPath) { EXPECT_TRUE(base::DeleteFileW(temp_path, false)); return; -}
\ No newline at end of file +} + +} // namespace sandbox |