From fbaf0bb26397aa498eb9156f06d5a6fe34dd7dd8 Mon Sep 17 00:00:00 2001 From: Daniel Baumann Date: Fri, 19 Apr 2024 03:14:29 +0200 Subject: Merging upstream version 125.0.1. Signed-off-by: Daniel Baumann --- .../fix_max_syscalls_linux_aarch64.patch | 25 + .../patches/with_update/patch_order.txt | 3 +- .../with_update/revert_remove_AddTargetPeer.patch | 310 --------- .../revert_remove_BrokerDuplicateHandle.patch | 743 --------------------- .../sandbox/linux/bpf_dsl/linux_syscall_ranges.h | 4 +- .../chromium/sandbox/win/src/broker_services.cc | 75 +-- .../chromium/sandbox/win/src/broker_services.h | 22 - .../chromium/sandbox/win/src/handle_dispatcher.cc | 93 --- .../chromium/sandbox/win/src/handle_dispatcher.h | 41 -- .../sandbox/win/src/handle_interception.cc | 48 -- .../chromium/sandbox/win/src/handle_interception.h | 24 - .../chromium/sandbox/win/src/handle_policy.cc | 93 --- .../chromium/sandbox/win/src/handle_policy.h | 39 -- .../chromium/sandbox/win/src/handle_policy_test.cc | 114 ---- .../sandbox/chromium/sandbox/win/src/ipc_tags.h | 1 - .../sandbox/chromium/sandbox/win/src/sandbox.h | 22 - .../chromium/sandbox/win/src/sandbox_policy.h | 4 - .../sandbox/win/src/sandbox_policy_base.cc | 9 - .../chromium/sandbox/win/src/target_services.cc | 10 - .../chromium/sandbox/win/src/target_services.h | 5 - .../sandbox/win/src/top_level_dispatcher.cc | 5 - security/sandbox/common/SandboxSettings.cpp | 5 +- security/sandbox/linux/SandboxInfo.cpp | 26 +- security/sandbox/linux/launch/SandboxLaunch.cpp | 25 +- security/sandbox/mac/Sandbox.mm | 2 + security/sandbox/moz.build | 3 - .../test/browser_content_sandbox_fs_snap.js | 2 +- .../sandbox/test/browser_content_sandbox_fs_xdg.js | 2 +- .../test/browser_content_sandbox_syscalls.js | 41 +- .../sandbox/test/browser_content_sandbox_utils.js | 22 +- .../win/src/sandboxbroker/sandboxBroker.cpp | 65 +- .../sandbox/win/src/sandboxbroker/sandboxBroker.h | 3 +- 32 files changed, 162 insertions(+), 1724 deletions(-) create mode 100644 security/sandbox/chromium-shim/patches/with_update/fix_max_syscalls_linux_aarch64.patch delete mode 100644 security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch delete mode 100644 security/sandbox/chromium-shim/patches/with_update/revert_remove_BrokerDuplicateHandle.patch delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_interception.cc delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_interception.h delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_policy.cc delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_policy.h delete mode 100644 security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc (limited to 'security/sandbox') diff --git a/security/sandbox/chromium-shim/patches/with_update/fix_max_syscalls_linux_aarch64.patch b/security/sandbox/chromium-shim/patches/with_update/fix_max_syscalls_linux_aarch64.patch new file mode 100644 index 0000000000..c5f816213a --- /dev/null +++ b/security/sandbox/chromium-shim/patches/with_update/fix_max_syscalls_linux_aarch64.patch @@ -0,0 +1,25 @@ +# HG changeset patch +# User Paul Bone +# Date 1708492973 -39600 +# Wed Feb 21 16:22:53 2024 +1100 +# Node ID 501cb36ee885ebd0939e1892f821d55ac149ceec +# Parent cf015b6f24b494190f562b255147f96e8b8b4139 +Bug 1866396 - Hard code the number of system calls for Linux on aarch64 r=jld + +Differential Revision: https://phabricator.services.mozilla.com/D202293 + +diff --git a/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h b/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h +--- a/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h ++++ b/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h +@@ -51,9 +51,9 @@ + + #elif defined(__aarch64__) + +-#include ++// The unistd.h included in the sysroot has a very old __NR_syscalls + #define MIN_SYSCALL 0u +-#define MAX_PUBLIC_SYSCALL __NR_syscalls ++#define MAX_PUBLIC_SYSCALL (MIN_SYSCALL + 1024u) + #define MAX_SYSCALL MAX_PUBLIC_SYSCALL + + #else diff --git a/security/sandbox/chromium-shim/patches/with_update/patch_order.txt b/security/sandbox/chromium-shim/patches/with_update/patch_order.txt index 8d40aeaa7f..028348a0f0 100755 --- a/security/sandbox/chromium-shim/patches/with_update/patch_order.txt +++ b/security/sandbox/chromium-shim/patches/with_update/patch_order.txt @@ -1,5 +1,3 @@ -revert_remove_AddTargetPeer.patch -revert_remove_BrokerDuplicateHandle.patch replace_ScopedNativeLibrary_in_ApplyMitigationsToCurrentThread.patch ifdef_out_FromStringInternal.patch add_option_to_not_use_restricting_sids.patch @@ -32,3 +30,4 @@ derive_sid_from_name.patch add_loongarch_defines.patch block_NtImpersonateAnonymousToken_before_LowerToken.patch fix_broker_alive_mutex.patch +fix_max_syscalls_linux_aarch64.patch diff --git a/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch b/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch deleted file mode 100644 index 04020b60b7..0000000000 --- a/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch +++ /dev/null @@ -1,310 +0,0 @@ -# HG changeset patch -# User Toshihito Kikuchi -# Date 1589671259 25200 -# Sat May 16 16:20:59 2020 -0700 -# Node ID 0b5183a01df78cc85264f2eae2c4d8e407bb1112 -# Parent d093cd9ccfcf06f4a1f0d7f1a4bd0f143ef92b4b -Add BrokerServicesBase::IsSafeDuplicationTarget. r=bobowen - -This patch adds BrokerServicesBase::IsSafeDuplicationTarget and -BrokerServicesBase::AddTargetPeer using the new ProcessTracker introduced by -https://chromium.googlesource.com/chromium/src.git/+/3d8382cf9dd44cf9c05e43e42c500f4825e1fed8 -We need these methods for HandlePolicy which is added as a different patch. - -Chromium used to have AddTargetPeer and IsActiveTarget, but removed by -the following commits because they were no longer used in Chromium. -https://chromium.googlesource.com/chromium/src.git/+/996b42db5296bd3d11b3d7fde1a4602bbcefed2c -https://chromium.googlesource.com/chromium/src.git/+/e615a1152ac6e10f1a91f0629fb8b5ca223ffbdc - -diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.cc b/security/sandbox/chromium/sandbox/win/src/broker_services.cc ---- a/security/sandbox/chromium/sandbox/win/src/broker_services.cc -+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.cc -@@ -154,16 +154,18 @@ namespace sandbox { - BrokerServicesBase::BrokerServicesBase() {} - - // The broker uses a dedicated worker thread that services the job completion - // port to perform policy notifications and associated cleanup tasks. - ResultCode BrokerServicesBase::Init() { - if (job_port_.IsValid() || thread_pool_) - return SBOX_ERROR_UNEXPECTED_CALL; - -+ ::InitializeCriticalSection(&lock_); -+ - job_port_.Set(::CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 0)); - if (!job_port_.IsValid()) - return SBOX_ERROR_CANNOT_INIT_BROKERSERVICES; - - no_targets_.Set(::CreateEventW(nullptr, true, false, nullptr)); - - job_thread_.Set(::CreateThread(nullptr, 0, // Default security and stack. - TargetEventsThread, this, 0, nullptr)); -@@ -191,16 +193,17 @@ BrokerServicesBase::~BrokerServicesBase( - - if (job_thread_.IsValid() && - WAIT_TIMEOUT == ::WaitForSingleObject(job_thread_.Get(), 1000)) { - // Cannot clean broker services. - NOTREACHED(); - return; - } - thread_pool_.reset(); -+ ::DeleteCriticalSection(&lock_); - } - - scoped_refptr BrokerServicesBase::CreatePolicy() { - // If you change the type of the object being created here you must also - // change the downcast to it in SpawnTarget(). - scoped_refptr policy(new PolicyBase); - // PolicyBase starts with refcount 1. - policy->Release(); -@@ -283,16 +286,21 @@ DWORD WINAPI BrokerServicesBase::TargetE - if (1 == target_counter) { - ::ResetEvent(no_targets); - } - break; - } - - case JOB_OBJECT_MSG_EXIT_PROCESS: - case JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS: { -+ { -+ AutoLock lock(&broker->lock_); -+ broker->active_targets_.erase( -+ static_cast(reinterpret_cast(ovl))); -+ } - size_t erase_result = child_process_ids.erase( - static_cast(reinterpret_cast(ovl))); - if (erase_result != 1U) { - // The process was untracked e.g. a child process of the target. - --untracked_target_counter; - DCHECK(untracked_target_counter >= 0); - } - --target_counter; -@@ -348,27 +356,31 @@ DWORD WINAPI BrokerServicesBase::TargetE - tracker->wait_handle = INVALID_HANDLE_VALUE; - } - processes.push_back(std::move(tracker)); - - } else if (THREAD_CTRL_PROCESS_SIGNALLED == key) { - ProcessTracker* tracker = - static_cast(reinterpret_cast(ovl)); - -+ { -+ AutoLock lock(&broker->lock_); -+ broker->active_targets_.erase(tracker->process_id); -+ } -+ - ::UnregisterWait(tracker->wait_handle); - tracker->wait_handle = INVALID_HANDLE_VALUE; - - // PID is unique until the process handle is closed in dtor. - processes.erase(std::remove_if(processes.begin(), processes.end(), - [&](auto&& p) -> bool { - return p->process_id == - tracker->process_id; - }), - processes.end()); -- - } else if (THREAD_CTRL_GET_POLICY_INFO == key) { - // Clone the policies for sandbox diagnostics. - std::unique_ptr receiver; - receiver.reset(static_cast( - reinterpret_cast(ovl))); - // The PollicyInfo ctor copies essential information from the trackers. - auto policy_list = std::make_unique(); - for (auto&& process_tracker : processes) { -@@ -637,47 +649,79 @@ ResultCode BrokerServicesBase::SpawnTarg - // the tracker. The worker thread takes ownership of these objects. - CHECK(::PostQueuedCompletionStatus( - job_port_.Get(), 0, THREAD_CTRL_NEW_JOB_TRACKER, - reinterpret_cast(tracker))); - // There is no obvious recovery after failure here. Previous version with - // SpawnCleanup() caused deletion of TargetProcess twice. crbug.com/480639 - CHECK( - AssociateCompletionPort(tracker->job.Get(), job_port_.Get(), tracker)); -+ -+ AutoLock lock(&lock_); -+ active_targets_.insert(process_info.process_id()); - } else { -- // Duplicate the process handle to give the tracking machinery -- // something valid to wait on in the tracking thread. -- HANDLE tmp_process_handle = INVALID_HANDLE_VALUE; -- if (!::DuplicateHandle(::GetCurrentProcess(), process_info.process_handle(), -- ::GetCurrentProcess(), &tmp_process_handle, -- SYNCHRONIZE, false, 0 /*no options*/)) { -- *last_error = ::GetLastError(); -+ result = AddTargetPeerInternal(process_info.process_handle(), -+ process_info.process_id(), -+ policy_base, last_error); -+ if (result != SBOX_ALL_OK) { - // This may fail in the same way as Job associated processes. - // crbug.com/480639. - SpawnCleanup(target); -- return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE; -+ return result; - } -- base::win::ScopedHandle dup_process_handle(tmp_process_handle); -- ProcessTracker* tracker = new ProcessTracker( -- policy_base, process_info.process_id(), std::move(dup_process_handle)); -- // The tracker and policy will leak if this call fails. -- ::PostQueuedCompletionStatus(job_port_.Get(), 0, -- THREAD_CTRL_NEW_PROCESS_TRACKER, -- reinterpret_cast(tracker)); - } - - *target_info = process_info.Take(); - return result; - } - - ResultCode BrokerServicesBase::WaitForAllTargets() { - ::WaitForSingleObject(no_targets_.Get(), INFINITE); - return SBOX_ALL_OK; - } - -+bool BrokerServicesBase::IsSafeDuplicationTarget(DWORD process_id) { -+ AutoLock lock(&lock_); -+ return active_targets_.find(process_id) != active_targets_.end(); -+} -+ -+ResultCode BrokerServicesBase::AddTargetPeerInternal( -+ HANDLE peer_process_handle, -+ DWORD peer_process_id, -+ scoped_refptr policy_base, -+ DWORD* last_error) { -+ // Duplicate the process handle to give the tracking machinery -+ // something valid to wait on in the tracking thread. -+ HANDLE tmp_process_handle = INVALID_HANDLE_VALUE; -+ if (!::DuplicateHandle(::GetCurrentProcess(), peer_process_handle, -+ ::GetCurrentProcess(), &tmp_process_handle, -+ SYNCHRONIZE, false, 0 /*no options*/)) { -+ *last_error = ::GetLastError(); -+ return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE; -+ } -+ base::win::ScopedHandle dup_process_handle(tmp_process_handle); -+ ProcessTracker* tracker = new ProcessTracker( -+ policy_base, peer_process_id, std::move(dup_process_handle)); -+ // The tracker and policy will leak if this call fails. -+ ::PostQueuedCompletionStatus(job_port_.Get(), 0, -+ THREAD_CTRL_NEW_PROCESS_TRACKER, -+ reinterpret_cast(tracker)); -+ -+ AutoLock lock(&lock_); -+ active_targets_.insert(peer_process_id); -+ -+ return SBOX_ALL_OK; -+} -+ -+ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) { -+ DWORD last_error; -+ return AddTargetPeerInternal(peer_process, ::GetProcessId(peer_process), -+ nullptr, &last_error); -+} -+ - ResultCode BrokerServicesBase::GetPolicyDiagnostics( - std::unique_ptr receiver) { - CHECK(job_thread_.IsValid()); - // Post to the job thread. - if (!::PostQueuedCompletionStatus( - job_port_.Get(), 0, THREAD_CTRL_GET_POLICY_INFO, - reinterpret_cast(receiver.get()))) { - receiver->OnError(SBOX_ERROR_GENERIC); -diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.h b/security/sandbox/chromium/sandbox/win/src/broker_services.h ---- a/security/sandbox/chromium/sandbox/win/src/broker_services.h -+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.h -@@ -13,16 +13,17 @@ - - #include "base/compiler_specific.h" - #include "base/macros.h" - #include "base/memory/scoped_refptr.h" - #include "base/win/scoped_handle.h" - #include "sandbox/win/src/crosscall_server.h" - #include "sandbox/win/src/job.h" - #include "sandbox/win/src/sandbox.h" -+#include "sandbox/win/src/sandbox_policy_base.h" - #include "sandbox/win/src/sharedmem_ipc_server.h" - #include "sandbox/win/src/win2k_threadpool.h" - #include "sandbox/win/src/win_utils.h" - - namespace sandbox { - - // BrokerServicesBase --------------------------------------------------------- - // Broker implementation version 0 -@@ -43,16 +44,24 @@ class BrokerServicesBase final : public - scoped_refptr CreatePolicy() override; - ResultCode SpawnTarget(const wchar_t* exe_path, - const wchar_t* command_line, - scoped_refptr policy, - ResultCode* last_warning, - DWORD* last_error, - PROCESS_INFORMATION* target) override; - ResultCode WaitForAllTargets() override; -+ ResultCode AddTargetPeer(HANDLE peer_process) override; -+ -+ // Checks if the supplied process ID matches one of the broker's active -+ // target processes. We use this method for the specific purpose of -+ // checking if we can safely duplicate a handle to the supplied process -+ // in DuplicateHandleProxyAction. -+ bool IsSafeDuplicationTarget(DWORD process_id); -+ - ResultCode GetPolicyDiagnostics( - std::unique_ptr receiver) override; - - private: - // The routine that the worker thread executes. It is in charge of - // notifications and cleanup-related tasks. - static DWORD WINAPI TargetEventsThread(PVOID param); - -@@ -65,14 +74,27 @@ class BrokerServicesBase final : public - base::win::ScopedHandle no_targets_; - - // Handle to the worker thread that reacts to job notifications. - base::win::ScopedHandle job_thread_; - - // Provides a pool of threads that are used to wait on the IPC calls. - std::unique_ptr thread_pool_; - -+ // The set representing the broker's active target processes including -+ // both sandboxed and unsandboxed peer processes. -+ std::set active_targets_; -+ -+ // Lock used to protect active_targets_ from being simultaneously accessed -+ // by multiple threads. -+ CRITICAL_SECTION lock_; -+ -+ ResultCode AddTargetPeerInternal(HANDLE peer_process_handle, -+ DWORD peer_process_id, -+ scoped_refptr policy_base, -+ DWORD* last_error); -+ - DISALLOW_COPY_AND_ASSIGN(BrokerServicesBase); - }; - - } // namespace sandbox - - #endif // SANDBOX_WIN_SRC_BROKER_SERVICES_H_ -diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox.h b/security/sandbox/chromium/sandbox/win/src/sandbox.h ---- a/security/sandbox/chromium/sandbox/win/src/sandbox.h -+++ b/security/sandbox/chromium/sandbox/win/src/sandbox.h -@@ -96,16 +96,24 @@ class BrokerServices { - - // This call blocks (waits) for all the targets to terminate. - // Returns: - // ALL_OK if successful. All other return values imply failure. - // If the return is ERROR_GENERIC, you can call ::GetLastError() to get - // more information. - virtual ResultCode WaitForAllTargets() = 0; - -+ // Adds an unsandboxed process as a peer for policy decisions (e.g. -+ // HANDLES_DUP_ANY policy). -+ // Returns: -+ // ALL_OK if successful. All other return values imply failure. -+ // If the return is ERROR_GENERIC, you can call ::GetLastError() to get -+ // more information. -+ virtual ResultCode AddTargetPeer(HANDLE peer_process) = 0; -+ - // This call creates a snapshot of policies managed by the sandbox and - // returns them via a helper class. - // Parameters: - // receiver: The |PolicyDiagnosticsReceiver| implementation will be - // called to accept the results of the call. - // Returns: - // ALL_OK if the request was dispatched. All other return values - // imply failure, and the responder will not receive its completion diff --git a/security/sandbox/chromium-shim/patches/with_update/revert_remove_BrokerDuplicateHandle.patch b/security/sandbox/chromium-shim/patches/with_update/revert_remove_BrokerDuplicateHandle.patch deleted file mode 100644 index 970c0d1db2..0000000000 --- a/security/sandbox/chromium-shim/patches/with_update/revert_remove_BrokerDuplicateHandle.patch +++ /dev/null @@ -1,743 +0,0 @@ -# HG changeset patch -# User Toshihito Kikuchi -# Date 1589671733 25200 -# Sat May 16 16:28:53 2020 -0700 -# Node ID 91bb5c3807cfe657cc24c9a3c217dd1f57db6d5c -# Parent 22eb0bf7180801edf775be44cf299a50e01eb7bf -Reinstate sandbox::TargetServices::BrokerDuplicateHandle. r=bobowen - -This patch reverts the commit removing sandbox::TargetServices::BrokerDuplicateHandle -and applies the new IpcTag type. - -https://chromium.googlesource.com/chromium/src.git/+/569193665184525ca366e65d0735f5c851106e43 -https://chromium.googlesource.com/chromium/src.git/+/c8cff7f9663ce6d1ef35e5c717f43c867c3906eb - -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc -@@ -0,0 +1,93 @@ -+// Copyright (c) 2012 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/handle_dispatcher.h" -+ -+#include -+ -+#include "base/win/scoped_handle.h" -+#include "sandbox/win/src/handle_interception.h" -+#include "sandbox/win/src/handle_policy.h" -+#include "sandbox/win/src/ipc_tags.h" -+#include "sandbox/win/src/policy_broker.h" -+#include "sandbox/win/src/policy_params.h" -+#include "sandbox/win/src/sandbox.h" -+#include "sandbox/win/src/sandbox_nt_util.h" -+#include "sandbox/win/src/sandbox_types.h" -+#include "sandbox/win/src/sandbox_utils.h" -+ -+namespace sandbox { -+ -+HandleDispatcher::HandleDispatcher(PolicyBase* policy_base) -+ : policy_base_(policy_base) { -+ static const IPCCall duplicate_handle_proxy = { -+ {IpcTag::DUPLICATEHANDLEPROXY, -+ {VOIDPTR_TYPE, UINT32_TYPE, UINT32_TYPE, UINT32_TYPE}}, -+ reinterpret_cast( -+ &HandleDispatcher::DuplicateHandleProxy)}; -+ -+ ipc_calls_.push_back(duplicate_handle_proxy); -+} -+ -+bool HandleDispatcher::SetupService(InterceptionManager* manager, -+ IpcTag service) { -+ // We perform no interceptions for handles right now. -+ switch (service) { -+ case IpcTag::DUPLICATEHANDLEPROXY: -+ return true; -+ -+ default: -+ return false; -+ } -+} -+ -+bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc, -+ HANDLE source_handle, -+ uint32_t target_process_id, -+ uint32_t desired_access, -+ uint32_t options) { -+ static NtQueryObject QueryObject = NULL; -+ if (!QueryObject) -+ ResolveNTFunctionPtr("NtQueryObject", &QueryObject); -+ -+ // Get a copy of the handle for use in the broker process. -+ HANDLE handle_temp; -+ if (!::DuplicateHandle(ipc->client_info->process, source_handle, -+ ::GetCurrentProcess(), &handle_temp, -+ 0, FALSE, DUPLICATE_SAME_ACCESS | options)) { -+ ipc->return_info.win32_result = ::GetLastError(); -+ return false; -+ } -+ options &= ~DUPLICATE_CLOSE_SOURCE; -+ base::win::ScopedHandle handle(handle_temp); -+ -+ // Get the object type (32 characters is safe; current max is 14). -+ BYTE buffer[sizeof(OBJECT_TYPE_INFORMATION) + 32 * sizeof(wchar_t)]; -+ OBJECT_TYPE_INFORMATION* type_info = -+ reinterpret_cast(buffer); -+ ULONG size = sizeof(buffer) - sizeof(wchar_t); -+ NTSTATUS error = -+ QueryObject(handle.Get(), ObjectTypeInformation, type_info, size, &size); -+ if (!NT_SUCCESS(error)) { -+ ipc->return_info.nt_status = error; -+ return false; -+ } -+ type_info->Name.Buffer[type_info->Name.Length / sizeof(wchar_t)] = L'\0'; -+ -+ CountedParameterSet params; -+ params[HandleTarget::NAME] = ParamPickerMake(type_info->Name.Buffer); -+ params[HandleTarget::TARGET] = ParamPickerMake(target_process_id); -+ -+ EvalResult eval = policy_base_->EvalPolicy(IpcTag::DUPLICATEHANDLEPROXY, -+ params.GetBase()); -+ ipc->return_info.win32_result = -+ HandlePolicy::DuplicateHandleProxyAction(eval, handle.Get(), -+ target_process_id, -+ &ipc->return_info.handle, -+ desired_access, options); -+ return true; -+} -+ -+} // namespace sandbox -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h -@@ -0,0 +1,41 @@ -+// Copyright (c) 2012 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_SRC_HANDLE_DISPATCHER_H_ -+#define SANDBOX_SRC_HANDLE_DISPATCHER_H_ -+ -+#include -+ -+#include "base/macros.h" -+#include "sandbox/win/src/crosscall_server.h" -+#include "sandbox/win/src/sandbox_policy_base.h" -+ -+namespace sandbox { -+ -+// This class handles handle-related IPC calls. -+class HandleDispatcher : public Dispatcher { -+ public: -+ explicit HandleDispatcher(PolicyBase* policy_base); -+ ~HandleDispatcher() override {} -+ -+ // Dispatcher interface. -+ bool SetupService(InterceptionManager* manager, IpcTag service) override; -+ -+ private: -+ // Processes IPC requests coming from calls to -+ // TargetServices::DuplicateHandle() in the target. -+ bool DuplicateHandleProxy(IPCInfo* ipc, -+ HANDLE source_handle, -+ uint32_t target_process_id, -+ uint32_t desired_access, -+ uint32_t options); -+ -+ PolicyBase* policy_base_; -+ DISALLOW_COPY_AND_ASSIGN(HandleDispatcher); -+}; -+ -+} // namespace sandbox -+ -+#endif // SANDBOX_SRC_HANDLE_DISPATCHER_H_ -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_interception.cc b/security/sandbox/chromium/sandbox/win/src/handle_interception.cc -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_interception.cc -@@ -0,0 +1,45 @@ -+// Copyright (c) 2012 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/handle_interception.h" -+ -+#include "sandbox/win/src/crosscall_client.h" -+#include "sandbox/win/src/ipc_tags.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 { -+ -+ResultCode DuplicateHandleProxy(HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options) { -+ *target_handle = NULL; -+ -+ void* memory = GetGlobalIPCMemory(); -+ if (NULL == memory) -+ return SBOX_ERROR_NO_SPACE; -+ -+ SharedMemIPCClient ipc(memory); -+ CrossCallReturn answer = {0}; -+ ResultCode code = CrossCall(ipc, IpcTag::DUPLICATEHANDLEPROXY, -+ source_handle, target_process_id, -+ desired_access, options, &answer); -+ if (SBOX_ALL_OK != code) -+ return code; -+ -+ if (answer.win32_result) { -+ ::SetLastError(answer.win32_result); -+ return SBOX_ERROR_GENERIC; -+ } -+ -+ *target_handle = answer.handle; -+ return SBOX_ALL_OK; -+} -+ -+} // namespace sandbox -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_interception.h b/security/sandbox/chromium/sandbox/win/src/handle_interception.h -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_interception.h -@@ -0,0 +1,24 @@ -+// Copyright (c) 2012 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/nt_internals.h" -+#include "sandbox/win/src/sandbox_types.h" -+ -+#ifndef SANDBOX_SRC_HANDLE_INTERCEPTION_H_ -+#define SANDBOX_SRC_HANDLE_INTERCEPTION_H_ -+ -+namespace sandbox { -+ -+// TODO(jschuh) Add an interception to catch dangerous DuplicateHandle calls. -+ -+ResultCode DuplicateHandleProxy(HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options); -+ -+} // namespace sandbox -+ -+#endif // SANDBOX_SRC_HANDLE_INTERCEPTION_H_ -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy.cc b/security/sandbox/chromium/sandbox/win/src/handle_policy.cc -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_policy.cc -@@ -0,0 +1,93 @@ -+// Copyright (c) 2012 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/handle_policy.h" -+ -+#include -+ -+#include "base/win/scoped_handle.h" -+#include "sandbox/win/src/broker_services.h" -+#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_types.h" -+#include "sandbox/win/src/sandbox_utils.h" -+ -+namespace sandbox { -+ -+bool HandlePolicy::GenerateRules(const wchar_t* type_name, -+ TargetPolicy::Semantics semantics, -+ LowLevelPolicy* policy) { -+ PolicyRule duplicate_rule(ASK_BROKER); -+ -+ switch (semantics) { -+ case TargetPolicy::HANDLES_DUP_ANY: { -+ if (!duplicate_rule.AddNumberMatch(IF_NOT, HandleTarget::TARGET, -+ ::GetCurrentProcessId(), EQUAL)) { -+ return false; -+ } -+ break; -+ } -+ -+ case TargetPolicy::HANDLES_DUP_BROKER: { -+ if (!duplicate_rule.AddNumberMatch(IF, HandleTarget::TARGET, -+ ::GetCurrentProcessId(), EQUAL)) { -+ return false; -+ } -+ break; -+ } -+ -+ default: -+ return false; -+ } -+ if (!duplicate_rule.AddStringMatch(IF, HandleTarget::NAME, type_name, -+ CASE_INSENSITIVE)) { -+ return false; -+ } -+ if (!policy->AddRule(IpcTag::DUPLICATEHANDLEPROXY, &duplicate_rule)) { -+ return false; -+ } -+ return true; -+} -+ -+DWORD HandlePolicy::DuplicateHandleProxyAction(EvalResult eval_result, -+ HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options) { -+ // The only action supported is ASK_BROKER which means duplicate the handle. -+ if (ASK_BROKER != eval_result) { -+ return ERROR_ACCESS_DENIED; -+ } -+ -+ base::win::ScopedHandle remote_target_process; -+ if (target_process_id != ::GetCurrentProcessId()) { -+ // Sandboxed children are dynamic, so we check that manually. -+ if (!BrokerServicesBase::GetInstance()->IsSafeDuplicationTarget( -+ target_process_id)) { -+ return ERROR_ACCESS_DENIED; -+ } -+ -+ remote_target_process.Set(::OpenProcess(PROCESS_DUP_HANDLE, FALSE, -+ target_process_id)); -+ if (!remote_target_process.IsValid()) -+ return ::GetLastError(); -+ } -+ -+ // If the policy didn't block us and we have no valid target, then the broker -+ // (this process) is the valid target. -+ HANDLE target_process = remote_target_process.IsValid() ? -+ remote_target_process.Get() : ::GetCurrentProcess(); -+ if (!::DuplicateHandle(::GetCurrentProcess(), source_handle, target_process, -+ target_handle, desired_access, FALSE, -+ options)) { -+ return ::GetLastError(); -+ } -+ -+ return ERROR_SUCCESS; -+} -+ -+} // namespace sandbox -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy.h b/security/sandbox/chromium/sandbox/win/src/handle_policy.h -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_policy.h -@@ -0,0 +1,39 @@ -+// Copyright (c) 2012 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_SRC_HANDLE_POLICY_H_ -+#define SANDBOX_SRC_HANDLE_POLICY_H_ -+ -+#include -+ -+#include "sandbox/win/src/crosscall_server.h" -+#include "sandbox/win/src/policy_low_level.h" -+#include "sandbox/win/src/sandbox_policy.h" -+ -+namespace sandbox { -+ -+enum EvalResult; -+ -+// This class centralizes most of the knowledge related to handle policy. -+class HandlePolicy { -+ public: -+ // Creates the required low-level policy rules to evaluate a high-level -+ // policy rule for handles, in particular duplicate action. -+ static bool GenerateRules(const wchar_t* type_name, -+ TargetPolicy::Semantics semantics, -+ LowLevelPolicy* policy); -+ -+ // Processes a 'TargetPolicy::DuplicateHandle()' request from the target. -+ static DWORD DuplicateHandleProxyAction(EvalResult eval_result, -+ HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options); -+}; -+ -+} // namespace sandbox -+ -+#endif // SANDBOX_SRC_HANDLE_POLICY_H_ -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc b/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc -new file mode 100644 ---- /dev/null -+++ b/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc -@@ -0,0 +1,114 @@ -+// Copyright (c) 2012 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 "base/strings/stringprintf.h" -+#include "sandbox/win/src/handle_policy.h" -+#include "sandbox/win/src/nt_internals.h" -+#include "sandbox/win/src/sandbox.h" -+#include "sandbox/win/src/sandbox_factory.h" -+#include "sandbox/win/src/sandbox_policy.h" -+#include "sandbox/win/src/win_utils.h" -+#include "sandbox/win/tests/common/controller.h" -+#include "testing/gtest/include/gtest/gtest.h" -+ -+namespace sandbox { -+ -+// Just waits for the supplied number of milliseconds. -+SBOX_TESTS_COMMAND int Handle_WaitProcess(int argc, wchar_t **argv) { -+ if (argc != 1) -+ return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; -+ -+ ::Sleep(::wcstoul(argv[0], NULL, 10)); -+ return SBOX_TEST_TIMED_OUT; -+} -+ -+// Attempts to duplicate an event handle into the target process. -+SBOX_TESTS_COMMAND int Handle_DuplicateEvent(int argc, wchar_t **argv) { -+ if (argc != 1) -+ return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; -+ -+ // Create a test event to use as a handle. -+ base::win::ScopedHandle test_event; -+ test_event.Set(::CreateEvent(NULL, TRUE, TRUE, NULL)); -+ if (!test_event.IsValid()) -+ return SBOX_TEST_FIRST_ERROR; -+ -+ // Get the target process ID. -+ DWORD target_process_id = ::wcstoul(argv[0], NULL, 10); -+ -+ HANDLE handle = NULL; -+ ResultCode result = SandboxFactory::GetTargetServices()->DuplicateHandle( -+ test_event.Get(), target_process_id, &handle, 0, DUPLICATE_SAME_ACCESS); -+ -+ return (result == SBOX_ALL_OK) ? SBOX_TEST_SUCCEEDED : SBOX_TEST_DENIED; -+} -+ -+// Tests that duplicating an object works only when the policy allows it. -+TEST(HandlePolicyTest, DuplicateHandle) { -+ TestRunner target; -+ TestRunner runner; -+ -+ // Kick off an asynchronous target process for testing. -+ target.SetAsynchronous(true); -+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"Handle_WaitProcess 30000")); -+ -+ // First test that we fail to open the event. -+ base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", -+ target.process_id()); -+ EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); -+ -+ // Now successfully open the event after adding a duplicate handle rule. -+ EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, -+ TargetPolicy::HANDLES_DUP_ANY, -+ L"Event")); -+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -+} -+ -+// Tests that duplicating an object works only when the policy allows it. -+TEST(HandlePolicyTest, DuplicatePeerHandle) { -+ TestRunner target; -+ TestRunner runner; -+ -+ // Kick off an asynchronous target process for testing. -+ target.SetAsynchronous(true); -+ target.SetUnsandboxed(true); -+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"Handle_WaitProcess 30000")); -+ -+ // First test that we fail to open the event. -+ base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", -+ target.process_id()); -+ EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); -+ -+ // Now successfully open the event after adding a duplicate handle rule. -+ EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, -+ TargetPolicy::HANDLES_DUP_ANY, -+ L"Event")); -+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -+} -+ -+// Tests that duplicating an object works only when the policy allows it. -+TEST(HandlePolicyTest, DuplicateBrokerHandle) { -+ TestRunner runner; -+ -+ // First test that we fail to open the event. -+ base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", -+ ::GetCurrentProcessId()); -+ EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); -+ -+ // Add the peer rule and make sure we fail again. -+ EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, -+ TargetPolicy::HANDLES_DUP_ANY, -+ L"Event")); -+ EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); -+ -+ -+ // Now successfully open the event after adding a broker handle rule. -+ EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, -+ TargetPolicy::HANDLES_DUP_BROKER, -+ L"Event")); -+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -+} -+ -+} // namespace sandbox -+ -diff --git a/security/sandbox/chromium/sandbox/win/src/ipc_tags.h b/security/sandbox/chromium/sandbox/win/src/ipc_tags.h ---- a/security/sandbox/chromium/sandbox/win/src/ipc_tags.h -+++ b/security/sandbox/chromium/sandbox/win/src/ipc_tags.h -@@ -23,16 +23,17 @@ enum class IpcTag { - NTOPENPROCESS, - NTOPENPROCESSTOKEN, - NTOPENPROCESSTOKENEX, - CREATEPROCESSW, - CREATEEVENT, - OPENEVENT, - NTCREATEKEY, - NTOPENKEY, -+ DUPLICATEHANDLEPROXY, - GDI_GDIDLLINITIALIZE, - GDI_GETSTOCKOBJECT, - USER_REGISTERCLASSW, - CREATETHREAD, - USER_ENUMDISPLAYMONITORS, - USER_ENUMDISPLAYDEVICES, - USER_GETMONITORINFO, - GDI_CREATEOPMPROTECTEDOUTPUTS, -diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox.h b/security/sandbox/chromium/sandbox/win/src/sandbox.h ---- a/security/sandbox/chromium/sandbox/win/src/sandbox.h -+++ b/security/sandbox/chromium/sandbox/win/src/sandbox.h -@@ -161,16 +161,30 @@ class TargetServices { - // fails the current process could be terminated immediately. - virtual void LowerToken() = 0; - - // Returns the ProcessState object. Through that object it's possible to have - // information about the current state of the process, such as whether - // LowerToken has been called or not. - virtual ProcessState* GetState() = 0; - -+ // Requests the broker to duplicate the supplied handle into the target -+ // process. The target process must be an active sandbox child process -+ // and the source process must have a corresponding policy allowing -+ // handle duplication for this object type. -+ // Returns: -+ // ALL_OK if successful. All other return values imply failure. -+ // If the return is ERROR_GENERIC, you can call ::GetLastError() to get -+ // more information. -+ virtual ResultCode DuplicateHandle(HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options) = 0; -+ - protected: - ~TargetServices() {} - }; - - class PolicyInfo { - public: - // Returns a JSON representation of the policy snapshot. - // This pointer has the same lifetime as this PolicyInfo object. -diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h ---- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h -+++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h -@@ -25,28 +25,32 @@ class TargetPolicy { - // 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_HANDLES, // Duplication of handles to other processes. - SUBSYS_WIN32K_LOCKDOWN, // Win32K Lockdown related policy. - SUBSYS_SIGNED_BINARY // Signed binary policy. - }; - - // Allowable semantics when a rule is matched. - enum Semantics { - FILES_ALLOW_ANY, // Allows open or create for any kind of access that - // the file system supports. - FILES_ALLOW_READONLY, // Allows open or create with read access only. - FILES_ALLOW_QUERY, // Allows access to query the attributes of a file. - FILES_ALLOW_DIR_ANY, // Allows open or create with directory semantics - // only. -+ HANDLES_DUP_ANY, // Allows duplicating handles opened with any -+ // access permissions. -+ HANDLES_DUP_BROKER, // Allows duplicating handles to the broker process. - NAMEDPIPES_ALLOW_ANY, // Allows creation of a named pipe. - PROCESS_MIN_EXEC, // Allows to create a process with minimal rights - // over the resulting process and thread handles. - // No other parameters besides the command line are - // passed to the child process. - PROCESS_ALL_EXEC, // Allows the creation of a process and return full - // access on the returned handles. - // This flag can be used only when the main token of -diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc ---- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc -+++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc -@@ -12,16 +12,17 @@ - #include "base/logging.h" - #include "base/macros.h" - #include "base/stl_util.h" - #include "base/strings/stringprintf.h" - #include "base/win/win_util.h" - #include "base/win/windows_version.h" - #include "sandbox/win/src/acl.h" - #include "sandbox/win/src/filesystem_policy.h" -+#include "sandbox/win/src/handle_policy.h" - #include "sandbox/win/src/interception.h" - #include "sandbox/win/src/job.h" - #include "sandbox/win/src/named_pipe_policy.h" - #include "sandbox/win/src/policy_broker.h" - #include "sandbox/win/src/policy_engine_processor.h" - #include "sandbox/win/src/policy_low_level.h" - #include "sandbox/win/src/process_mitigations.h" - #include "sandbox/win/src/process_mitigations_win32k_policy.h" -@@ -754,16 +755,24 @@ ResultCode PolicyBase::AddRuleInternal(S - } - case SUBSYS_REGISTRY: { - if (!RegistryPolicy::GenerateRules(pattern, semantics, policy_maker_)) { - NOTREACHED(); - return SBOX_ERROR_BAD_PARAMS; - } - break; - } -+ case SUBSYS_HANDLES: { -+ if (!HandlePolicy::GenerateRules(pattern, semantics, policy_maker_)) { -+ NOTREACHED(); -+ return SBOX_ERROR_BAD_PARAMS; -+ } -+ break; -+ } -+ - case SUBSYS_WIN32K_LOCKDOWN: { - // Win32k intercept rules only supported on Windows 8 and above. This must - // match the version checks in process_mitigations.cc for consistency. - if (base::win::GetVersion() >= base::win::Version::WIN8) { - DCHECK_EQ(MITIGATION_WIN32K_DISABLE, - mitigations_ & MITIGATION_WIN32K_DISABLE) - << "Enable MITIGATION_WIN32K_DISABLE before adding win32k policy " - "rules."; -diff --git a/security/sandbox/chromium/sandbox/win/src/target_services.cc b/security/sandbox/chromium/sandbox/win/src/target_services.cc ---- a/security/sandbox/chromium/sandbox/win/src/target_services.cc -+++ b/security/sandbox/chromium/sandbox/win/src/target_services.cc -@@ -7,16 +7,17 @@ - #include - - #include - #include - - #include "base/win/windows_version.h" - #include "sandbox/win/src/crosscall_client.h" - #include "sandbox/win/src/handle_closer_agent.h" -+#include "sandbox/win/src/handle_interception.h" - #include "sandbox/win/src/heap_helper.h" - #include "sandbox/win/src/ipc_tags.h" - #include "sandbox/win/src/process_mitigations.h" - #include "sandbox/win/src/restricted_token_utils.h" - #include "sandbox/win/src/sandbox.h" - #include "sandbox/win/src/sandbox_nt_util.h" - #include "sandbox/win/src/sandbox_types.h" - #include "sandbox/win/src/sharedmem_ipc_client.h" -@@ -239,9 +240,19 @@ void ProcessState::SetRevertedToSelf() { - if (process_state_ < ProcessStateInternal::REVERTED_TO_SELF) - process_state_ = ProcessStateInternal::REVERTED_TO_SELF; - } - - void ProcessState::SetCsrssConnected(bool csrss_connected) { - csrss_connected_ = csrss_connected; - } - -+ -+ResultCode TargetServicesBase::DuplicateHandle(HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options) { -+ return sandbox::DuplicateHandleProxy(source_handle, target_process_id, -+ target_handle, desired_access, options); -+} -+ - } // namespace sandbox -diff --git a/security/sandbox/chromium/sandbox/win/src/target_services.h b/security/sandbox/chromium/sandbox/win/src/target_services.h ---- a/security/sandbox/chromium/sandbox/win/src/target_services.h -+++ b/security/sandbox/chromium/sandbox/win/src/target_services.h -@@ -40,16 +40,21 @@ class ProcessState { - class TargetServicesBase : public TargetServices { - public: - TargetServicesBase(); - - // Public interface of TargetServices. - ResultCode Init() override; - void LowerToken() override; - ProcessState* GetState() override; -+ ResultCode DuplicateHandle(HANDLE source_handle, -+ DWORD target_process_id, -+ HANDLE* target_handle, -+ DWORD desired_access, -+ DWORD options) override; - - // Factory method. - static TargetServicesBase* GetInstance(); - - // Sends a simple IPC Message that has a well-known answer. Returns true - // if the IPC was successful and false otherwise. There are 2 versions of - // this test: 1 and 2. The first one send a simple message while the - // second one send a message with an in/out param. -diff --git a/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc ---- a/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc -+++ b/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc -@@ -5,16 +5,17 @@ - #include "sandbox/win/src/top_level_dispatcher.h" - - #include - #include - - #include "base/logging.h" - #include "sandbox/win/src/crosscall_server.h" - #include "sandbox/win/src/filesystem_dispatcher.h" -+#include "sandbox/win/src/handle_dispatcher.h" - #include "sandbox/win/src/interception.h" - #include "sandbox/win/src/internal_types.h" - #include "sandbox/win/src/ipc_tags.h" - #include "sandbox/win/src/named_pipe_dispatcher.h" - #include "sandbox/win/src/process_mitigations_win32k_dispatcher.h" - #include "sandbox/win/src/process_thread_dispatcher.h" - #include "sandbox/win/src/registry_dispatcher.h" - #include "sandbox/win/src/sandbox_policy_base.h" -@@ -55,16 +56,20 @@ TopLevelDispatcher::TopLevelDispatcher(P - ipc_targets_[static_cast(IpcTag::OPENEVENT)] = dispatcher; - sync_dispatcher_.reset(dispatcher); - - dispatcher = new RegistryDispatcher(policy_); - ipc_targets_[static_cast(IpcTag::NTCREATEKEY)] = dispatcher; - ipc_targets_[static_cast(IpcTag::NTOPENKEY)] = dispatcher; - registry_dispatcher_.reset(dispatcher); - -+ dispatcher = new HandleDispatcher(policy_); -+ ipc_targets_[static_cast(IpcTag::DUPLICATEHANDLEPROXY)] = dispatcher; -+ handle_dispatcher_.reset(dispatcher); -+ - dispatcher = new ProcessMitigationsWin32KDispatcher(policy_); - ipc_targets_[static_cast(IpcTag::GDI_GDIDLLINITIALIZE)] = dispatcher; - ipc_targets_[static_cast(IpcTag::GDI_GETSTOCKOBJECT)] = dispatcher; - ipc_targets_[static_cast(IpcTag::USER_REGISTERCLASSW)] = dispatcher; - ipc_targets_[static_cast(IpcTag::USER_ENUMDISPLAYMONITORS)] = - dispatcher; - ipc_targets_[static_cast(IpcTag::USER_ENUMDISPLAYDEVICES)] = - dispatcher; diff --git a/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h b/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h index 313511f22e..c43e73448f 100644 --- a/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h +++ b/security/sandbox/chromium/sandbox/linux/bpf_dsl/linux_syscall_ranges.h @@ -51,9 +51,9 @@ #elif defined(__aarch64__) -#include +// The unistd.h included in the sysroot has a very old __NR_syscalls #define MIN_SYSCALL 0u -#define MAX_PUBLIC_SYSCALL __NR_syscalls +#define MAX_PUBLIC_SYSCALL (MIN_SYSCALL + 1024u) #define MAX_SYSCALL MAX_PUBLIC_SYSCALL #else diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.cc b/security/sandbox/chromium/sandbox/win/src/broker_services.cc index 0ba71bbd5d..613becf37b 100644 --- a/security/sandbox/chromium/sandbox/win/src/broker_services.cc +++ b/security/sandbox/chromium/sandbox/win/src/broker_services.cc @@ -159,8 +159,6 @@ ResultCode BrokerServicesBase::Init() { if (job_port_.IsValid() || thread_pool_) return SBOX_ERROR_UNEXPECTED_CALL; - ::InitializeCriticalSection(&lock_); - job_port_.Set(::CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 0)); if (!job_port_.IsValid()) return SBOX_ERROR_CANNOT_INIT_BROKERSERVICES; @@ -201,7 +199,6 @@ BrokerServicesBase::~BrokerServicesBase() { return; } thread_pool_.reset(); - ::DeleteCriticalSection(&lock_); } scoped_refptr BrokerServicesBase::CreatePolicy() { @@ -294,11 +291,6 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) { case JOB_OBJECT_MSG_EXIT_PROCESS: case JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS: { - { - AutoLock lock(&broker->lock_); - broker->active_targets_.erase( - static_cast(reinterpret_cast(ovl))); - } size_t erase_result = child_process_ids.erase( static_cast(reinterpret_cast(ovl))); if (erase_result != 1U) { @@ -364,11 +356,6 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) { ProcessTracker* tracker = static_cast(reinterpret_cast(ovl)); - { - AutoLock lock(&broker->lock_); - broker->active_targets_.erase(tracker->process_id); - } - ::UnregisterWait(tracker->wait_handle); tracker->wait_handle = INVALID_HANDLE_VALUE; // Copy process_id so that we can legally reference it even after we have @@ -659,19 +646,26 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, // SpawnCleanup() caused deletion of TargetProcess twice. crbug.com/480639 CHECK( AssociateCompletionPort(tracker->job.Get(), job_port_.Get(), tracker)); - - AutoLock lock(&lock_); - active_targets_.insert(process_info.process_id()); } else { - result = AddTargetPeerInternal(process_info.process_handle(), - process_info.process_id(), - policy_base, last_error); - if (result != SBOX_ALL_OK) { + // Duplicate the process handle to give the tracking machinery + // something valid to wait on in the tracking thread. + HANDLE tmp_process_handle = INVALID_HANDLE_VALUE; + if (!::DuplicateHandle(::GetCurrentProcess(), process_info.process_handle(), + ::GetCurrentProcess(), &tmp_process_handle, + SYNCHRONIZE, false, 0 /*no options*/)) { + *last_error = ::GetLastError(); // This may fail in the same way as Job associated processes. // crbug.com/480639. target->Terminate(); - return result; + return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE; } + base::win::ScopedHandle dup_process_handle(tmp_process_handle); + ProcessTracker* tracker = new ProcessTracker( + policy_base, process_info.process_id(), std::move(dup_process_handle)); + // The tracker and policy will leak if this call fails. + ::PostQueuedCompletionStatus(job_port_.Get(), 0, + THREAD_CTRL_NEW_PROCESS_TRACKER, + reinterpret_cast(tracker)); } *target_info = process_info.Take(); @@ -683,45 +677,6 @@ ResultCode BrokerServicesBase::WaitForAllTargets() { return SBOX_ALL_OK; } -bool BrokerServicesBase::IsSafeDuplicationTarget(DWORD process_id) { - AutoLock lock(&lock_); - return active_targets_.find(process_id) != active_targets_.end(); -} - -ResultCode BrokerServicesBase::AddTargetPeerInternal( - HANDLE peer_process_handle, - DWORD peer_process_id, - scoped_refptr policy_base, - DWORD* last_error) { - // Duplicate the process handle to give the tracking machinery - // something valid to wait on in the tracking thread. - HANDLE tmp_process_handle = INVALID_HANDLE_VALUE; - if (!::DuplicateHandle(::GetCurrentProcess(), peer_process_handle, - ::GetCurrentProcess(), &tmp_process_handle, - SYNCHRONIZE, false, 0 /*no options*/)) { - *last_error = ::GetLastError(); - return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE; - } - base::win::ScopedHandle dup_process_handle(tmp_process_handle); - ProcessTracker* tracker = new ProcessTracker( - policy_base, peer_process_id, std::move(dup_process_handle)); - // The tracker and policy will leak if this call fails. - ::PostQueuedCompletionStatus(job_port_.Get(), 0, - THREAD_CTRL_NEW_PROCESS_TRACKER, - reinterpret_cast(tracker)); - - AutoLock lock(&lock_); - active_targets_.insert(peer_process_id); - - return SBOX_ALL_OK; -} - -ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) { - DWORD last_error; - return AddTargetPeerInternal(peer_process, ::GetProcessId(peer_process), - nullptr, &last_error); -} - ResultCode BrokerServicesBase::GetPolicyDiagnostics( std::unique_ptr receiver) { CHECK(job_thread_.IsValid()); diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.h b/security/sandbox/chromium/sandbox/win/src/broker_services.h index 64dc6d66e5..1d7eafdea3 100644 --- a/security/sandbox/chromium/sandbox/win/src/broker_services.h +++ b/security/sandbox/chromium/sandbox/win/src/broker_services.h @@ -19,7 +19,6 @@ #include "sandbox/win/src/crosscall_server.h" #include "sandbox/win/src/job.h" #include "sandbox/win/src/sandbox.h" -#include "sandbox/win/src/sandbox_policy_base.h" #include "sandbox/win/src/sharedmem_ipc_server.h" #include "sandbox/win/src/win2k_threadpool.h" #include "sandbox/win/src/win_utils.h" @@ -51,14 +50,6 @@ class BrokerServicesBase final : public BrokerServices, DWORD* last_error, PROCESS_INFORMATION* target) override; ResultCode WaitForAllTargets() override; - ResultCode AddTargetPeer(HANDLE peer_process) override; - - // Checks if the supplied process ID matches one of the broker's active - // target processes. We use this method for the specific purpose of - // checking if we can safely duplicate a handle to the supplied process - // in DuplicateHandleProxyAction. - bool IsSafeDuplicationTarget(DWORD process_id); - ResultCode GetPolicyDiagnostics( std::unique_ptr receiver) override; @@ -84,19 +75,6 @@ class BrokerServicesBase final : public BrokerServices, // Provides a pool of threads that are used to wait on the IPC calls. std::unique_ptr thread_pool_; - // The set representing the broker's active target processes including - // both sandboxed and unsandboxed peer processes. - std::set active_targets_; - - // Lock used to protect active_targets_ from being simultaneously accessed - // by multiple threads. - CRITICAL_SECTION lock_; - - ResultCode AddTargetPeerInternal(HANDLE peer_process_handle, - DWORD peer_process_id, - scoped_refptr policy_base, - DWORD* last_error); - DISALLOW_COPY_AND_ASSIGN(BrokerServicesBase); }; diff --git a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc deleted file mode 100644 index 611e33d2a6..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.cc +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (c) 2012 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/handle_dispatcher.h" - -#include - -#include "base/win/scoped_handle.h" -#include "sandbox/win/src/handle_interception.h" -#include "sandbox/win/src/handle_policy.h" -#include "sandbox/win/src/ipc_tags.h" -#include "sandbox/win/src/policy_broker.h" -#include "sandbox/win/src/policy_params.h" -#include "sandbox/win/src/sandbox.h" -#include "sandbox/win/src/sandbox_nt_util.h" -#include "sandbox/win/src/sandbox_types.h" -#include "sandbox/win/src/sandbox_utils.h" - -namespace sandbox { - -HandleDispatcher::HandleDispatcher(PolicyBase* policy_base) - : policy_base_(policy_base) { - static const IPCCall duplicate_handle_proxy = { - {IpcTag::DUPLICATEHANDLEPROXY, - {VOIDPTR_TYPE, UINT32_TYPE, UINT32_TYPE, UINT32_TYPE}}, - reinterpret_cast( - &HandleDispatcher::DuplicateHandleProxy)}; - - ipc_calls_.push_back(duplicate_handle_proxy); -} - -bool HandleDispatcher::SetupService(InterceptionManager* manager, - IpcTag service) { - // We perform no interceptions for handles right now. - switch (service) { - case IpcTag::DUPLICATEHANDLEPROXY: - return true; - - default: - return false; - } -} - -bool HandleDispatcher::DuplicateHandleProxy(IPCInfo* ipc, - HANDLE source_handle, - uint32_t target_process_id, - uint32_t desired_access, - uint32_t options) { - static NtQueryObject QueryObject = NULL; - if (!QueryObject) - ResolveNTFunctionPtr("NtQueryObject", &QueryObject); - - // Get a copy of the handle for use in the broker process. - HANDLE handle_temp; - if (!::DuplicateHandle(ipc->client_info->process, source_handle, - ::GetCurrentProcess(), &handle_temp, - 0, FALSE, DUPLICATE_SAME_ACCESS | options)) { - ipc->return_info.win32_result = ::GetLastError(); - return false; - } - options &= ~DUPLICATE_CLOSE_SOURCE; - base::win::ScopedHandle handle(handle_temp); - - // Get the object type (32 characters is safe; current max is 14). - BYTE buffer[sizeof(OBJECT_TYPE_INFORMATION) + 32 * sizeof(wchar_t)]; - OBJECT_TYPE_INFORMATION* type_info = - reinterpret_cast(buffer); - ULONG size = sizeof(buffer) - sizeof(wchar_t); - NTSTATUS error = - QueryObject(handle.Get(), ObjectTypeInformation, type_info, size, &size); - if (!NT_SUCCESS(error)) { - ipc->return_info.nt_status = error; - return false; - } - type_info->Name.Buffer[type_info->Name.Length / sizeof(wchar_t)] = L'\0'; - - CountedParameterSet params; - params[HandleTarget::NAME] = ParamPickerMake(type_info->Name.Buffer); - params[HandleTarget::TARGET] = ParamPickerMake(target_process_id); - - EvalResult eval = policy_base_->EvalPolicy(IpcTag::DUPLICATEHANDLEPROXY, - params.GetBase()); - ipc->return_info.win32_result = - HandlePolicy::DuplicateHandleProxyAction(eval, handle.Get(), - target_process_id, - &ipc->return_info.handle, - desired_access, options); - return true; -} - -} // namespace sandbox - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h b/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h deleted file mode 100644 index 6f9adbc10b..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_dispatcher.h +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2012 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_SRC_HANDLE_DISPATCHER_H_ -#define SANDBOX_SRC_HANDLE_DISPATCHER_H_ - -#include - -#include "base/macros.h" -#include "sandbox/win/src/crosscall_server.h" -#include "sandbox/win/src/sandbox_policy_base.h" - -namespace sandbox { - -// This class handles handle-related IPC calls. -class HandleDispatcher : public Dispatcher { - public: - explicit HandleDispatcher(PolicyBase* policy_base); - ~HandleDispatcher() override {} - - // Dispatcher interface. - bool SetupService(InterceptionManager* manager, IpcTag service) override; - - private: - // Processes IPC requests coming from calls to - // TargetServices::DuplicateHandle() in the target. - bool DuplicateHandleProxy(IPCInfo* ipc, - HANDLE source_handle, - uint32_t target_process_id, - uint32_t desired_access, - uint32_t options); - - PolicyBase* policy_base_; - DISALLOW_COPY_AND_ASSIGN(HandleDispatcher); -}; - -} // namespace sandbox - -#endif // SANDBOX_SRC_HANDLE_DISPATCHER_H_ - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_interception.cc b/security/sandbox/chromium/sandbox/win/src/handle_interception.cc deleted file mode 100644 index 53db4a8b27..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_interception.cc +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (c) 2012 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/handle_interception.h" - -#include "sandbox/win/src/crosscall_client.h" -#include "sandbox/win/src/ipc_tags.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" -#include "mozilla/sandboxing/sandboxLogging.h" - -namespace sandbox { - -ResultCode DuplicateHandleProxy(HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options) { - *target_handle = NULL; - - void* memory = GetGlobalIPCMemory(); - if (NULL == memory) - return SBOX_ERROR_NO_SPACE; - - SharedMemIPCClient ipc(memory); - CrossCallReturn answer = {0}; - ResultCode code = CrossCall(ipc, IpcTag::DUPLICATEHANDLEPROXY, - source_handle, target_process_id, - desired_access, options, &answer); - if (SBOX_ALL_OK != code) - return code; - - 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; -} - -} // namespace sandbox - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_interception.h b/security/sandbox/chromium/sandbox/win/src/handle_interception.h deleted file mode 100644 index 6f60811f17..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_interception.h +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) 2012 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/nt_internals.h" -#include "sandbox/win/src/sandbox_types.h" - -#ifndef SANDBOX_SRC_HANDLE_INTERCEPTION_H_ -#define SANDBOX_SRC_HANDLE_INTERCEPTION_H_ - -namespace sandbox { - -// TODO(jschuh) Add an interception to catch dangerous DuplicateHandle calls. - -ResultCode DuplicateHandleProxy(HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options); - -} // namespace sandbox - -#endif // SANDBOX_SRC_HANDLE_INTERCEPTION_H_ - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy.cc b/security/sandbox/chromium/sandbox/win/src/handle_policy.cc deleted file mode 100644 index fa3295ae3f..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_policy.cc +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (c) 2012 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/handle_policy.h" - -#include - -#include "base/win/scoped_handle.h" -#include "sandbox/win/src/broker_services.h" -#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_types.h" -#include "sandbox/win/src/sandbox_utils.h" - -namespace sandbox { - -bool HandlePolicy::GenerateRules(const wchar_t* type_name, - TargetPolicy::Semantics semantics, - LowLevelPolicy* policy) { - PolicyRule duplicate_rule(ASK_BROKER); - - switch (semantics) { - case TargetPolicy::HANDLES_DUP_ANY: { - if (!duplicate_rule.AddNumberMatch(IF_NOT, HandleTarget::TARGET, - ::GetCurrentProcessId(), EQUAL)) { - return false; - } - break; - } - - case TargetPolicy::HANDLES_DUP_BROKER: { - if (!duplicate_rule.AddNumberMatch(IF, HandleTarget::TARGET, - ::GetCurrentProcessId(), EQUAL)) { - return false; - } - break; - } - - default: - return false; - } - if (!duplicate_rule.AddStringMatch(IF, HandleTarget::NAME, type_name, - CASE_INSENSITIVE)) { - return false; - } - if (!policy->AddRule(IpcTag::DUPLICATEHANDLEPROXY, &duplicate_rule)) { - return false; - } - return true; -} - -DWORD HandlePolicy::DuplicateHandleProxyAction(EvalResult eval_result, - HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options) { - // The only action supported is ASK_BROKER which means duplicate the handle. - if (ASK_BROKER != eval_result) { - return ERROR_ACCESS_DENIED; - } - - base::win::ScopedHandle remote_target_process; - if (target_process_id != ::GetCurrentProcessId()) { - // Sandboxed children are dynamic, so we check that manually. - if (!BrokerServicesBase::GetInstance()->IsSafeDuplicationTarget( - target_process_id)) { - return ERROR_ACCESS_DENIED; - } - - remote_target_process.Set(::OpenProcess(PROCESS_DUP_HANDLE, FALSE, - target_process_id)); - if (!remote_target_process.IsValid()) - return ::GetLastError(); - } - - // If the policy didn't block us and we have no valid target, then the broker - // (this process) is the valid target. - HANDLE target_process = remote_target_process.IsValid() ? - remote_target_process.Get() : ::GetCurrentProcess(); - if (!::DuplicateHandle(::GetCurrentProcess(), source_handle, target_process, - target_handle, desired_access, FALSE, - options)) { - return ::GetLastError(); - } - - return ERROR_SUCCESS; -} - -} // namespace sandbox - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy.h b/security/sandbox/chromium/sandbox/win/src/handle_policy.h deleted file mode 100644 index 29ce5ab666..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_policy.h +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) 2012 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_SRC_HANDLE_POLICY_H_ -#define SANDBOX_SRC_HANDLE_POLICY_H_ - -#include - -#include "sandbox/win/src/crosscall_server.h" -#include "sandbox/win/src/policy_low_level.h" -#include "sandbox/win/src/sandbox_policy.h" - -namespace sandbox { - -enum EvalResult; - -// This class centralizes most of the knowledge related to handle policy. -class HandlePolicy { - public: - // Creates the required low-level policy rules to evaluate a high-level - // policy rule for handles, in particular duplicate action. - static bool GenerateRules(const wchar_t* type_name, - TargetPolicy::Semantics semantics, - LowLevelPolicy* policy); - - // Processes a 'TargetPolicy::DuplicateHandle()' request from the target. - static DWORD DuplicateHandleProxyAction(EvalResult eval_result, - HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options); -}; - -} // namespace sandbox - -#endif // SANDBOX_SRC_HANDLE_POLICY_H_ - diff --git a/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc b/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc deleted file mode 100644 index 11382da811..0000000000 --- a/security/sandbox/chromium/sandbox/win/src/handle_policy_test.cc +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright (c) 2012 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 "base/strings/stringprintf.h" -#include "sandbox/win/src/handle_policy.h" -#include "sandbox/win/src/nt_internals.h" -#include "sandbox/win/src/sandbox.h" -#include "sandbox/win/src/sandbox_factory.h" -#include "sandbox/win/src/sandbox_policy.h" -#include "sandbox/win/src/win_utils.h" -#include "sandbox/win/tests/common/controller.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace sandbox { - -// Just waits for the supplied number of milliseconds. -SBOX_TESTS_COMMAND int Handle_WaitProcess(int argc, wchar_t **argv) { - if (argc != 1) - return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - - ::Sleep(::wcstoul(argv[0], NULL, 10)); - return SBOX_TEST_TIMED_OUT; -} - -// Attempts to duplicate an event handle into the target process. -SBOX_TESTS_COMMAND int Handle_DuplicateEvent(int argc, wchar_t **argv) { - if (argc != 1) - return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - - // Create a test event to use as a handle. - base::win::ScopedHandle test_event; - test_event.Set(::CreateEvent(NULL, TRUE, TRUE, NULL)); - if (!test_event.IsValid()) - return SBOX_TEST_FIRST_ERROR; - - // Get the target process ID. - DWORD target_process_id = ::wcstoul(argv[0], NULL, 10); - - HANDLE handle = NULL; - ResultCode result = SandboxFactory::GetTargetServices()->DuplicateHandle( - test_event.Get(), target_process_id, &handle, 0, DUPLICATE_SAME_ACCESS); - - return (result == SBOX_ALL_OK) ? SBOX_TEST_SUCCEEDED : SBOX_TEST_DENIED; -} - -// Tests that duplicating an object works only when the policy allows it. -TEST(HandlePolicyTest, DuplicateHandle) { - TestRunner target; - TestRunner runner; - - // Kick off an asynchronous target process for testing. - target.SetAsynchronous(true); - EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"Handle_WaitProcess 30000")); - - // First test that we fail to open the event. - base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", - target.process_id()); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); - - // Now successfully open the event after adding a duplicate handle rule. - EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, - TargetPolicy::HANDLES_DUP_ANY, - L"Event")); - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -} - -// Tests that duplicating an object works only when the policy allows it. -TEST(HandlePolicyTest, DuplicatePeerHandle) { - TestRunner target; - TestRunner runner; - - // Kick off an asynchronous target process for testing. - target.SetAsynchronous(true); - target.SetUnsandboxed(true); - EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"Handle_WaitProcess 30000")); - - // First test that we fail to open the event. - base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", - target.process_id()); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); - - // Now successfully open the event after adding a duplicate handle rule. - EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, - TargetPolicy::HANDLES_DUP_ANY, - L"Event")); - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -} - -// Tests that duplicating an object works only when the policy allows it. -TEST(HandlePolicyTest, DuplicateBrokerHandle) { - TestRunner runner; - - // First test that we fail to open the event. - base::string16 cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d", - ::GetCurrentProcessId()); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); - - // Add the peer rule and make sure we fail again. - EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, - TargetPolicy::HANDLES_DUP_ANY, - L"Event")); - EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str())); - - - // Now successfully open the event after adding a broker handle rule. - EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES, - TargetPolicy::HANDLES_DUP_BROKER, - L"Event")); - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str())); -} - -} // namespace sandbox - diff --git a/security/sandbox/chromium/sandbox/win/src/ipc_tags.h b/security/sandbox/chromium/sandbox/win/src/ipc_tags.h index ec6de4a66a..e655fc4b9a 100644 --- a/security/sandbox/chromium/sandbox/win/src/ipc_tags.h +++ b/security/sandbox/chromium/sandbox/win/src/ipc_tags.h @@ -28,7 +28,6 @@ enum class IpcTag { OPENEVENT, NTCREATEKEY, NTOPENKEY, - DUPLICATEHANDLEPROXY, GDI_GDIDLLINITIALIZE, GDI_GETSTOCKOBJECT, USER_REGISTERCLASSW, diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox.h b/security/sandbox/chromium/sandbox/win/src/sandbox.h index 858c350558..6133687f48 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox.h +++ b/security/sandbox/chromium/sandbox/win/src/sandbox.h @@ -102,14 +102,6 @@ class BrokerServices { // more information. virtual ResultCode WaitForAllTargets() = 0; - // Adds an unsandboxed process as a peer for policy decisions (e.g. - // HANDLES_DUP_ANY policy). - // Returns: - // ALL_OK if successful. All other return values imply failure. - // If the return is ERROR_GENERIC, you can call ::GetLastError() to get - // more information. - virtual ResultCode AddTargetPeer(HANDLE peer_process) = 0; - // This call creates a snapshot of policies managed by the sandbox and // returns them via a helper class. // Parameters: @@ -172,20 +164,6 @@ class TargetServices { // LowerToken has been called or not. virtual ProcessState* GetState() = 0; - // Requests the broker to duplicate the supplied handle into the target - // process. The target process must be an active sandbox child process - // and the source process must have a corresponding policy allowing - // handle duplication for this object type. - // Returns: - // ALL_OK if successful. All other return values imply failure. - // If the return is ERROR_GENERIC, you can call ::GetLastError() to get - // more information. - virtual ResultCode DuplicateHandle(HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options) = 0; - virtual ResultCode GetComplexLineBreaks(const WCHAR* text, uint32_t length, uint8_t* break_before) = 0; diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h index 75514ef595..10a29d6f3b 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h +++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy.h @@ -30,7 +30,6 @@ class TargetPolicy { SUBSYS_PROCESS, // Creation of child processes. SUBSYS_REGISTRY, // Creation and opening of registry keys. SUBSYS_SYNC, // Creation of named sync objects. - SUBSYS_HANDLES, // Duplication of handles to other processes. SUBSYS_WIN32K_LOCKDOWN, // Win32K Lockdown related policy. SUBSYS_SIGNED_BINARY, // Signed binary policy. SUBSYS_LINE_BREAK // Complex line break policy. @@ -44,9 +43,6 @@ class TargetPolicy { FILES_ALLOW_QUERY, // Allows access to query the attributes of a file. FILES_ALLOW_DIR_ANY, // Allows open or create with directory semantics // only. - HANDLES_DUP_ANY, // Allows duplicating handles opened with any - // access permissions. - HANDLES_DUP_BROKER, // Allows duplicating handles to the broker process. NAMEDPIPES_ALLOW_ANY, // Allows creation of a named pipe. PROCESS_MIN_EXEC, // Allows to create a process with minimal rights // over the resulting process and thread handles. diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc index f228dbbc31..0a23cb4470 100644 --- a/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc +++ b/security/sandbox/chromium/sandbox/win/src/sandbox_policy_base.cc @@ -17,7 +17,6 @@ #include "base/win/windows_version.h" #include "sandbox/win/src/acl.h" #include "sandbox/win/src/filesystem_policy.h" -#include "sandbox/win/src/handle_policy.h" #include "sandbox/win/src/interception.h" #include "sandbox/win/src/job.h" #include "sandbox/win/src/line_break_policy.h" @@ -775,14 +774,6 @@ ResultCode PolicyBase::AddRuleInternal(SubSystem subsystem, } break; } - case SUBSYS_HANDLES: { - if (!HandlePolicy::GenerateRules(pattern, semantics, policy_maker_)) { - NOTREACHED(); - return SBOX_ERROR_BAD_PARAMS; - } - break; - } - case SUBSYS_WIN32K_LOCKDOWN: { // Win32k intercept rules only supported on Windows 8 and above. This must // match the version checks in process_mitigations.cc for consistency. diff --git a/security/sandbox/chromium/sandbox/win/src/target_services.cc b/security/sandbox/chromium/sandbox/win/src/target_services.cc index a80e0106ef..7bd0e87aab 100644 --- a/security/sandbox/chromium/sandbox/win/src/target_services.cc +++ b/security/sandbox/chromium/sandbox/win/src/target_services.cc @@ -12,7 +12,6 @@ #include "base/win/windows_version.h" #include "sandbox/win/src/crosscall_client.h" #include "sandbox/win/src/handle_closer_agent.h" -#include "sandbox/win/src/handle_interception.h" #include "sandbox/win/src/heap_helper.h" #include "sandbox/win/src/line_break_interception.h" #include "sandbox/win/src/ipc_tags.h" @@ -246,15 +245,6 @@ void ProcessState::SetCsrssConnected(bool csrss_connected) { csrss_connected_ = csrss_connected; } -ResultCode TargetServicesBase::DuplicateHandle(HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options) { - return sandbox::DuplicateHandleProxy(source_handle, target_process_id, - target_handle, desired_access, options); -} - ResultCode TargetServicesBase::GetComplexLineBreaks(const WCHAR* text, uint32_t length, uint8_t* break_before) { diff --git a/security/sandbox/chromium/sandbox/win/src/target_services.h b/security/sandbox/chromium/sandbox/win/src/target_services.h index 1d70d4cd34..0231a250f3 100644 --- a/security/sandbox/chromium/sandbox/win/src/target_services.h +++ b/security/sandbox/chromium/sandbox/win/src/target_services.h @@ -45,11 +45,6 @@ class TargetServicesBase : public TargetServices { ResultCode Init() override; void LowerToken() override; ProcessState* GetState() override; - ResultCode DuplicateHandle(HANDLE source_handle, - DWORD target_process_id, - HANDLE* target_handle, - DWORD desired_access, - DWORD options) override; ResultCode GetComplexLineBreaks(const WCHAR* text, uint32_t length, uint8_t* break_before) final; diff --git a/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc b/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc index 3c8f8e25e5..7c072d5279 100644 --- a/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc +++ b/security/sandbox/chromium/sandbox/win/src/top_level_dispatcher.cc @@ -10,7 +10,6 @@ #include "base/logging.h" #include "sandbox/win/src/crosscall_server.h" #include "sandbox/win/src/filesystem_dispatcher.h" -#include "sandbox/win/src/handle_dispatcher.h" #include "sandbox/win/src/interception.h" #include "sandbox/win/src/internal_types.h" #include "sandbox/win/src/ipc_tags.h" @@ -62,10 +61,6 @@ TopLevelDispatcher::TopLevelDispatcher(PolicyBase* policy) : policy_(policy) { ipc_targets_[static_cast(IpcTag::NTOPENKEY)] = dispatcher; registry_dispatcher_.reset(dispatcher); - dispatcher = new HandleDispatcher(policy_); - ipc_targets_[static_cast(IpcTag::DUPLICATEHANDLEPROXY)] = dispatcher; - handle_dispatcher_.reset(dispatcher); - dispatcher = new ProcessMitigationsWin32KDispatcher(policy_); ipc_targets_[static_cast(IpcTag::GDI_GDIDLLINITIALIZE)] = dispatcher; ipc_targets_[static_cast(IpcTag::GDI_GETSTOCKOBJECT)] = dispatcher; diff --git a/security/sandbox/common/SandboxSettings.cpp b/security/sandbox/common/SandboxSettings.cpp index b0b24bf7d0..d44413da0b 100644 --- a/security/sandbox/common/SandboxSettings.cpp +++ b/security/sandbox/common/SandboxSettings.cpp @@ -104,10 +104,9 @@ nsIXULRuntime::ContentWin32kLockdownState GetContentWin32kLockdownState() { static auto getLockdownState = [] { auto state = GetWin32kLockdownState(); - const char* stateStr = ContentWin32kLockdownStateToString(state); - CrashReporter::AnnotateCrashReport( + CrashReporter::RecordAnnotationCString( CrashReporter::Annotation::ContentSandboxWin32kState, - nsDependentCString(stateStr)); + ContentWin32kLockdownStateToString(state)); return state; }; diff --git a/security/sandbox/linux/SandboxInfo.cpp b/security/sandbox/linux/SandboxInfo.cpp index 3d71e55921..aefd9dac2d 100644 --- a/security/sandbox/linux/SandboxInfo.cpp +++ b/security/sandbox/linux/SandboxInfo.cpp @@ -134,11 +134,21 @@ static bool CanCreateUserNamespace() { // the new capabilities (in this case, cloning another namespace) to // detect AppArmor policies that allow CLONE_NEWUSER but don't allow // doing anything useful with it. - pid_t pid = syscall(__NR_clone, SIGCHLD | CLONE_NEWUSER | CLONE_NEWPID, - nullptr, nullptr, nullptr, nullptr); + // + // Bug 1884347: There's a new AppArmor feature which can result in + // unsharing NEWUSER and NEWPID (or NEWNET etc.) in one syscall + // being allowed, but further use of capabilities will be blocked + // afterwards. That may be a bug, but we need to handle it. + pid_t pid = syscall(__NR_clone, SIGCHLD | CLONE_NEWUSER, nullptr, nullptr, + nullptr, nullptr); if (pid == 0) { - // In the child. Do as little as possible. - _exit(0); + // The exact meaning of `unshare(CLONE_NEWPID)` is slightly + // counterintuitive but in this case it doesn't matter. This just + // needs to be some operation that attempts to use capabilities, + // to check if it's blocked by an LSM. + int rv = unshare(CLONE_NEWPID); + // Exit with status 0 on success, 1 on failure. + _exit(rv == 0 ? 0 : 1); } if (pid == -1) { // Failure. @@ -149,11 +159,17 @@ static bool CanCreateUserNamespace() { return false; } // Otherwise, in the parent and successful. - bool waitpid_ok = HANDLE_EINTR(waitpid(pid, nullptr, 0)) == pid; + int wstatus; + bool waitpid_ok = HANDLE_EINTR(waitpid(pid, &wstatus, 0)) == pid; MOZ_ASSERT(waitpid_ok); if (!waitpid_ok) { return false; } + // Check for failures reported by the child process. + if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) { + setenv(kCacheEnvName, "0", 1); + return false; + } setenv(kCacheEnvName, "1", 1); return true; } diff --git a/security/sandbox/linux/launch/SandboxLaunch.cpp b/security/sandbox/linux/launch/SandboxLaunch.cpp index bec94f3c4c..6617ff475d 100644 --- a/security/sandbox/linux/launch/SandboxLaunch.cpp +++ b/security/sandbox/linux/launch/SandboxLaunch.cpp @@ -511,6 +511,8 @@ static pid_t ForkWithFlags(int aFlags) { return ret; } +// Returns true for success, or returns false and sets errno on +// failure. Intended only for procfs pseudo-files. static bool WriteStringToFile(const char* aPath, const char* aStr, const size_t aLen) { int fd = open(aPath, O_WRONLY); @@ -519,6 +521,11 @@ static bool WriteStringToFile(const char* aPath, const char* aStr, } ssize_t written = write(fd, aStr, aLen); if (close(fd) != 0 || written != ssize_t(aLen)) { + // procfs shouldn't ever cause a short write, but ensure that + // errno is set to something distinctive if it does + if (written >= 0) { + errno = EMSGSIZE; + } return false; } return true; @@ -537,6 +544,7 @@ static void ConfigureUserNamespace(uid_t uid, gid_t gid) { len = static_cast(SafeSPrintf(buf, "%d %d 1", uid, uid)); MOZ_RELEASE_ASSERT(len < sizeof(buf)); if (!WriteStringToFile("/proc/self/uid_map", buf, len)) { + SANDBOX_LOG_ERRNO("writing /proc/self/uid_map"); MOZ_CRASH("Failed to write /proc/self/uid_map"); } @@ -549,6 +557,7 @@ static void ConfigureUserNamespace(uid_t uid, gid_t gid) { len = static_cast(SafeSPrintf(buf, "%d %d 1", gid, gid)); MOZ_RELEASE_ASSERT(len < sizeof(buf)); if (!WriteStringToFile("/proc/self/gid_map", buf, len)) { + SANDBOX_LOG_ERRNO("writing /proc/self/gid_map"); MOZ_CRASH("Failed to write /proc/self/gid_map"); } } @@ -641,6 +650,9 @@ void SandboxLaunch::StartChrootServer() { char msg; ssize_t msgLen = HANDLE_EINTR(read(mChrootServer, &msg, 1)); + if (msgLen < 0) { + SANDBOX_LOG_ERRNO("chroot server couldn't read request"); + } if (msgLen == 0) { // Process exited before chrooting (or chose not to chroot?). _exit(0); @@ -653,7 +665,10 @@ void SandboxLaunch::StartChrootServer() { // exits at the end of this function, and which is always // unwriteable. int rv = chroot("/proc/self/fdinfo"); - MOZ_RELEASE_ASSERT(rv == 0); + if (rv != 0) { + SANDBOX_LOG_ERRNO("chroot"); + MOZ_CRASH("chroot failed"); + } // Drop CAP_SYS_CHROOT ASAP. This must happen before responding; // the main child won't be able to waitpid(), so it could start @@ -664,10 +679,16 @@ void SandboxLaunch::StartChrootServer() { // remove that. (Note: if the process can obtain directory fds, for // example via SandboxBroker, it must be blocked from using fchdir.) rv = chdir("/"); - MOZ_RELEASE_ASSERT(rv == 0); + if (rv != 0) { + SANDBOX_LOG_ERRNO("chdir(\"/\")"); + MOZ_CRASH("chdir(\"/\") failed"); + } msg = kSandboxChrootResponse; msgLen = HANDLE_EINTR(write(mChrootServer, &msg, 1)); + if (msgLen < 0) { + SANDBOX_LOG_ERRNO("chroot server couldn't send response"); + } MOZ_RELEASE_ASSERT(msgLen == 1); _exit(0); } diff --git a/security/sandbox/mac/Sandbox.mm b/security/sandbox/mac/Sandbox.mm index 2c9972a8fa..e4167a335a 100644 --- a/security/sandbox/mac/Sandbox.mm +++ b/security/sandbox/mac/Sandbox.mm @@ -303,11 +303,13 @@ bool StartMacSandbox(MacSandboxInfo const& aInfo, std::string& aErrorMessage) { // Nothing to do here specifically break; +#ifdef MOZ_APPLEMEDIA case ipc::SandboxingKind::UTILITY_AUDIO_DECODING_APPLE_MEDIA: { profile.append(SandboxPolicyUtilityAudioDecoderAppleMediaAddend); params.push_back("MAC_OS_VERSION"); params.push_back(combinedVersion.c_str()); } break; +#endif default: MOZ_ASSERT(false, "Invalid SandboxingKind"); diff --git a/security/sandbox/moz.build b/security/sandbox/moz.build index 806af7e58b..468573f220 100644 --- a/security/sandbox/moz.build +++ b/security/sandbox/moz.build @@ -113,9 +113,6 @@ elif CONFIG["OS_ARCH"] == "WINNT": "chromium/sandbox/win/src/filesystem_policy.cc", "chromium/sandbox/win/src/handle_closer.cc", "chromium/sandbox/win/src/handle_closer_agent.cc", - "chromium/sandbox/win/src/handle_dispatcher.cc", - "chromium/sandbox/win/src/handle_interception.cc", - "chromium/sandbox/win/src/handle_policy.cc", "chromium/sandbox/win/src/heap_helper.cc", "chromium/sandbox/win/src/interception.cc", "chromium/sandbox/win/src/interception_agent.cc", diff --git a/security/sandbox/test/browser_content_sandbox_fs_snap.js b/security/sandbox/test/browser_content_sandbox_fs_snap.js index a8b26a1e31..06f04c1d33 100644 --- a/security/sandbox/test/browser_content_sandbox_fs_snap.js +++ b/security/sandbox/test/browser_content_sandbox_fs_snap.js @@ -18,7 +18,7 @@ Services.scriptloader.loadSubScript( add_task(async function () { // Ensure that SNAP is there const snap = Services.env.get("SNAP"); - ok(snap.length > 1, "SNAP is defined"); + Assert.greater(snap.length, 1, "SNAP is defined"); // If it is there, do actual testing sanityChecks(); diff --git a/security/sandbox/test/browser_content_sandbox_fs_xdg.js b/security/sandbox/test/browser_content_sandbox_fs_xdg.js index f5150fc329..34dee9d1a0 100644 --- a/security/sandbox/test/browser_content_sandbox_fs_xdg.js +++ b/security/sandbox/test/browser_content_sandbox_fs_xdg.js @@ -18,7 +18,7 @@ Services.scriptloader.loadSubScript( add_task(async function () { // Ensure that XDG_CONFIG_HOME is there const xdgConfigHome = Services.env.get("XDG_CONFIG_HOME"); - ok(xdgConfigHome.length > 1, "XDG_CONFIG_HOME is defined"); + Assert.greater(xdgConfigHome.length, 1, "XDG_CONFIG_HOME is defined"); // If it is there, do actual testing sanityChecks(); diff --git a/security/sandbox/test/browser_content_sandbox_syscalls.js b/security/sandbox/test/browser_content_sandbox_syscalls.js index dab47cf356..71d3c7ad12 100644 --- a/security/sandbox/test/browser_content_sandbox_syscalls.js +++ b/security/sandbox/test/browser_content_sandbox_syscalls.js @@ -262,7 +262,7 @@ add_task(async function () { } info(`security.sandbox.content.level=${level}`); - ok(level > 0, "content sandbox is enabled."); + Assert.greater(level, 0, "content sandbox is enabled."); let areSyscallsSandboxed = areContentSyscallsSandboxed(level); @@ -282,7 +282,7 @@ add_task(async function () { // exec something harmless, this should fail let cmd = getOSExecCmd(); let rv = await SpecialPowers.spawn(browser, [{ lib, cmd }], callExec); - ok(rv == -1, `exec(${cmd}) is not permitted`); + Assert.equal(rv, -1, `exec(${cmd}) is not permitted`); } // use open syscall @@ -295,7 +295,7 @@ add_task(async function () { [{ lib, path, flags }], callOpen ); - ok(fd < 0, "opening a file for writing in home is not permitted"); + Assert.less(fd, 0, "opening a file for writing in home is not permitted"); } // use open syscall @@ -311,19 +311,24 @@ add_task(async function () { callOpen ); if (isMac()) { - ok( - fd === -1, + Assert.strictEqual( + fd, + -1, "opening a file for writing in content temp is not permitted" ); } else { - ok(fd >= 0, "opening a file for writing in content temp is permitted"); + Assert.greaterOrEqual( + fd, + 0, + "opening a file for writing in content temp is permitted" + ); } } // use fork syscall if (isLinux() || isMac()) { let rv = await SpecialPowers.spawn(browser, [{ lib }], callFork); - ok(rv == -1, "calling fork is not permitted"); + Assert.equal(rv, -1, "calling fork is not permitted"); } // On macOS before 10.10 the |sysctl-name| predicate didn't exist for @@ -336,21 +341,21 @@ add_task(async function () { [{ lib, name: "kern.boottime" }], callSysctl ); - ok(rv == -1, "calling sysctl('kern.boottime') is not permitted"); + Assert.equal(rv, -1, "calling sysctl('kern.boottime') is not permitted"); rv = await SpecialPowers.spawn( browser, [{ lib, name: "net.inet.ip.ttl" }], callSysctl ); - ok(rv == -1, "calling sysctl('net.inet.ip.ttl') is not permitted"); + Assert.equal(rv, -1, "calling sysctl('net.inet.ip.ttl') is not permitted"); rv = await SpecialPowers.spawn( browser, [{ lib, name: "hw.ncpu" }], callSysctl ); - ok(rv == 0, "calling sysctl('hw.ncpu') is permitted"); + Assert.equal(rv, 0, "calling sysctl('hw.ncpu') is permitted"); } if (isLinux()) { @@ -359,7 +364,11 @@ add_task(async function () { // verify we block PR_CAPBSET_READ with EINVAL let option = lazy.LIBC.PR_CAPBSET_READ; let rv = await SpecialPowers.spawn(browser, [{ lib, option }], callPrctl); - ok(rv === lazy.LIBC.EINVAL, "prctl(PR_CAPBSET_READ) is blocked"); + Assert.strictEqual( + rv, + lazy.LIBC.EINVAL, + "prctl(PR_CAPBSET_READ) is blocked" + ); const kernelVersion = await getKernelVersion(); const glibcVersion = getGlibcVersion(); @@ -375,8 +384,9 @@ add_task(async function () { [{ lib, dirfd, path, mode, flag: 0x01 }], callFaccessat2 ); - ok( - rv === lazy.LIBC.ENOSYS, + Assert.strictEqual( + rv, + lazy.LIBC.ENOSYS, "faccessat2 (flag=0x01) was blocked with ENOSYS" ); @@ -385,8 +395,9 @@ add_task(async function () { [{ lib, dirfd, path, mode, flag: lazy.LIBC.AT_EACCESS }], callFaccessat2 ); - ok( - rv === lazy.LIBC.EACCES, + Assert.strictEqual( + rv, + lazy.LIBC.EACCES, "faccessat2 (flag=0x200) was allowed, errno=EACCES" ); } else { diff --git a/security/sandbox/test/browser_content_sandbox_utils.js b/security/sandbox/test/browser_content_sandbox_utils.js index ce6ed39ff6..9b4c4af70a 100644 --- a/security/sandbox/test/browser_content_sandbox_utils.js +++ b/security/sandbox/test/browser_content_sandbox_utils.js @@ -33,7 +33,7 @@ function sanityChecks() { } info(`security.sandbox.content.level=${level}`); - ok(level > 0, "content sandbox is enabled."); + Assert.greater(level, 0, "content sandbox is enabled."); let isFileIOSandboxed = isContentFileIOSandboxed(level); @@ -234,7 +234,7 @@ function isContentFileIOSandboxed(level) { // Returns the lowest sandbox level where blanket reading of the profile // directory from the content process should be blocked by the sandbox. -function minProfileReadSandboxLevel(level) { +function minProfileReadSandboxLevel() { switch (Services.appinfo.OS) { case "WINNT": return 3; @@ -250,7 +250,7 @@ function minProfileReadSandboxLevel(level) { // Returns the lowest sandbox level where blanket reading of the home // directory from the content process should be blocked by the sandbox. -function minHomeReadSandboxLevel(level) { +function minHomeReadSandboxLevel() { switch (Services.appinfo.OS) { case "WINNT": return 3; @@ -391,8 +391,9 @@ function GetBrowserType(type) { } browserType = GetBrowserType[type]; - ok( - browserType.remoteType === type, + Assert.strictEqual( + browserType.remoteType, + type, `GetBrowserType(${type}) returns a ${type} process` ); return browserType; @@ -445,8 +446,9 @@ async function runTestsList(tests) { test.func ); - ok( - result.ok == test.ok, + Assert.equal( + result.ok, + test.ok, `reading ${test.desc} from a ${processType} process ` + `is ${okString} (${test.file.path})` ); @@ -454,7 +456,11 @@ async function runTestsList(tests) { // if the directory is not expected to be readable, // ensure the listing has zero entries if (test.func === readDir && !test.ok) { - ok(result.numEntries == 0, `directory list is empty (${test.file.path})`); + Assert.equal( + result.numEntries, + 0, + `directory list is empty (${test.file.path})` + ); } if (test.cleanup != undefined) { diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp index 9b9ed97262..3e2ce617bd 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp @@ -30,7 +30,6 @@ #include "mozilla/Telemetry.h" #include "mozilla/WinDllServices.h" #include "mozilla/WindowsVersion.h" -#include "mozilla/WinHeaderOnlyUtils.h" #include "mozilla/ipc/LaunchError.h" #include "nsAppDirectoryServiceDefs.h" #include "nsCOMPtr.h" @@ -116,11 +115,27 @@ static sandbox::ResultCode AddWin32kLockdownPolicy( return result; } +static void CacheDirAndAutoClear(const nsAString& aDir, + StaticAutoPtr* cacheVar) { + *cacheVar = new nsString(aDir); + ClearOnShutdown(cacheVar); + + // Convert network share path to format for sandbox policy. + if (Substring(**cacheVar, 0, 2).Equals(u"\\\\"_ns)) { + (*cacheVar)->InsertLiteral(u"??\\UNC", 1); + } +} + /* static */ -void SandboxBroker::Initialize(sandbox::BrokerServices* aBrokerServices) { +void SandboxBroker::Initialize(sandbox::BrokerServices* aBrokerServices, + const nsAString& aBinDir) { sBrokerService = aBrokerServices; sRunningFromNetworkDrive = widget::WinUtils::RunningFromANetworkDrive(); + + if (!aBinDir.IsEmpty()) { + CacheDirAndAutoClear(aBinDir, &sBinDir); + } } static void CacheDirAndAutoClear(nsIProperties* aDirSvc, const char* aDirKey, @@ -135,14 +150,9 @@ static void CacheDirAndAutoClear(nsIProperties* aDirSvc, const char* aDirKey, return; } - *cacheVar = new nsString(); - ClearOnShutdown(cacheVar); - MOZ_ALWAYS_SUCCEEDS(dirToCache->GetPath(**cacheVar)); - - // Convert network share path to format for sandbox policy. - if (Substring(**cacheVar, 0, 2).Equals(u"\\\\"_ns)) { - (*cacheVar)->InsertLiteral(u"??\\UNC", 1); - } + nsAutoString dirPath; + MOZ_ALWAYS_SUCCEEDS(dirToCache->GetPath(dirPath)); + CacheDirAndAutoClear(dirPath, cacheVar); } /* static */ @@ -166,7 +176,6 @@ void SandboxBroker::GeckoDependentInitialize() { return; } - CacheDirAndAutoClear(dirSvc, NS_GRE_DIR, &sBinDir); CacheDirAndAutoClear(dirSvc, NS_APP_USER_PROFILE_50_DIR, &sProfileDir); CacheDirAndAutoClear(dirSvc, NS_WIN_LOCAL_APPDATA_DIR, &sLocalAppDataDir); #ifdef ENABLE_SYSTEM_EXTENSION_DIRS @@ -481,33 +490,11 @@ static sandbox::ResultCode AllowProxyLoadFromBinDir( sandbox::TargetPolicy* aPolicy) { // Allow modules in the directory containing the executable such as // mozglue.dll, nss3.dll, etc. - static UniquePtr sInstallDir; - if (!sInstallDir) { - // Since this function can be called before sBinDir is initialized, - // we cache the install path by ourselves. - UniquePtr appDirStr; - if (GetInstallDirectory(appDirStr)) { - sInstallDir = MakeUnique(appDirStr.get()); - sInstallDir->Append(u"\\*"); - - auto setClearOnShutdown = [ptr = &sInstallDir]() -> void { - ClearOnShutdown(ptr); - }; - if (NS_IsMainThread()) { - setClearOnShutdown(); - } else { - SchedulerGroup::Dispatch(NS_NewRunnableFunction( - "InitSignedPolicyRulesToBypassCig", std::move(setClearOnShutdown))); - } - } - - if (!sInstallDir) { - return sandbox::SBOX_ERROR_GENERIC; - } - } + nsAutoString rulePath(*sBinDir); + rulePath.Append(u"\\*"_ns); return aPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_SIGNED_BINARY, sandbox::TargetPolicy::SIGNED_ALLOW_LOAD, - sInstallDir->get()); + rulePath.get()); } static sandbox::ResultCode AddCigToPolicy( @@ -1084,6 +1071,10 @@ void SandboxBroker::SetSecurityLevelForGPUProcess(int32_t aSandboxLevel) { sandbox::TargetPolicy::FILES_ALLOW_ANY, L"\\??\\pipe\\gecko-crash-server-pipe.*")); + // Add rule to allow read access to installation directory. + AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_READONLY, + sBinDir, u"\\*"_ns); + // The GPU process needs to write to a shader cache for performance reasons if (sProfileDir) { AddCachedDirRule(mPolicy, sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY, @@ -1833,8 +1824,6 @@ void SandboxBroker::ApplyLoggingPolicy() { L"HKEY_CURRENT_USER\\dummy"); mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_SYNC, sandbox::TargetPolicy::EVENTS_ALLOW_READONLY, L"dummy"); - mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_HANDLES, - sandbox::TargetPolicy::HANDLES_DUP_BROKER, L"dummy"); } SandboxBroker::~SandboxBroker() { diff --git a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h index 45a0cbb38b..40b3cf1501 100644 --- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h +++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h @@ -75,7 +75,8 @@ class SandboxBroker : public AbstractSandboxBroker { public: SandboxBroker(); - static void Initialize(sandbox::BrokerServices* aBrokerServices); + static void Initialize(sandbox::BrokerServices* aBrokerServices, + const nsAString& aBinDir); static void EnsureLpacPermsissionsOnDir(const nsString& aDir); -- cgit v1.2.3