morgan pushed to branch tor-browser-128.8.0esr-14.0-1 at The Tor Project / 
Applications / Tor Browser


Commits:
cb0f7b5c by Yannis Juglaret at 2025-03-26T20:28:42+00:00
Bug 1956398 - Avoid duplicating pseudo-handles in ipc_channel_win.cc. r=nika 
a=RyanVM

Differential Revision: https://phabricator.services.mozilla.com/D243135

- - - - -


1 changed file:

- ipc/chromium/src/chrome/common/ipc_channel_win.cc


Changes:

=====================================
ipc/chromium/src/chrome/common/ipc_channel_win.cc
=====================================
@@ -30,6 +30,34 @@
 
 using namespace mozilla::ipc;
 
+namespace {
+
+// This logic is borrowed from Chromium's `base/win/win_util.h`. It allows us
+// to distinguish pseudo-handle values, such as returned by GetCurrentProcess()
+// (-1), GetCurrentThread() (-2), and potentially more. The code there claims
+// that fuzzers have found issues up until -12 with DuplicateHandle.
+//
+// 
https://source.chromium.org/chromium/chromium/src/+/36dbbf38697dd1e23ef8944bb9e57f6e0b3d41ec:base/win/win_util.h
+inline bool IsPseudoHandle(HANDLE handle) {
+  auto handleValue = static_cast<int32_t>(reinterpret_cast<uintptr_t>(handle));
+  return -12 <= handleValue && handleValue < 0;
+}
+
+// A real handle is a handle that is not a pseudo-handle. Always preferably use
+// this variant over ::DuplicateHandle. Only use stock ::DuplicateHandle if you
+// explicitly need the ability to duplicate a pseudo-handle.
+inline bool DuplicateRealHandle(HANDLE source_process, HANDLE source_handle,
+                                HANDLE target_process, LPHANDLE target_handle,
+                                DWORD desired_access, BOOL inherit_handle,
+                                DWORD options) {
+  MOZ_RELEASE_ASSERT(!IsPseudoHandle(source_handle));
+  return static_cast<bool>(::DuplicateHandle(
+      source_process, source_handle, target_process, target_handle,
+      desired_access, inherit_handle, options));
+}
+
+}  // namespace
+
 namespace IPC {
 
//------------------------------------------------------------------------------
 
@@ -595,7 +623,7 @@ bool Channel::ChannelImpl::AcceptHandles(Message& msg) {
   nsTArray<mozilla::UniqueFileHandle> handles(num_handles);
   for (uint32_t handleValue : payload) {
     HANDLE ipc_handle = Uint32ToHandle(handleValue);
-    if (!ipc_handle || ipc_handle == INVALID_HANDLE_VALUE) {
+    if (!ipc_handle || IsPseudoHandle(ipc_handle)) {
       CHROMIUM_LOG(ERROR)
           << "Attempt to accept invalid or null handle from process "
           << other_pid_ << " for message " << msg.name() << " in 
AcceptHandles";
@@ -613,9 +641,10 @@ bool Channel::ChannelImpl::AcceptHandles(Message& msg) {
         CHROMIUM_LOG(ERROR) << "other_process_ is invalid in AcceptHandles";
         return false;
       }
-      if (!::DuplicateHandle(other_process_, ipc_handle, GetCurrentProcess(),
-                             getter_Transfers(local_handle), 0, FALSE,
-                             DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) {
+      if (!DuplicateRealHandle(
+              other_process_, ipc_handle, GetCurrentProcess(),
+              getter_Transfers(local_handle), 0, FALSE,
+              DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) {
         DWORD err = GetLastError();
         // Don't log out a scary looking error if this failed due to the target
         // process terminating.
@@ -688,9 +717,9 @@ bool Channel::ChannelImpl::TransferHandles(Message& msg) {
         CHROMIUM_LOG(ERROR) << "other_process_ is invalid in TransferHandles";
         return false;
       }
-      if (!::DuplicateHandle(GetCurrentProcess(), local_handle.get(),
-                             other_process_, &ipc_handle, 0, FALSE,
-                             DUPLICATE_SAME_ACCESS)) {
+      if (!DuplicateRealHandle(GetCurrentProcess(), local_handle.get(),
+                               other_process_, &ipc_handle, 0, FALSE,
+                               DUPLICATE_SAME_ACCESS)) {
         DWORD err = GetLastError();
         // Don't log out a scary looking error if this failed due to the target
         // process terminating.



View it on GitLab: 
https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/cb0f7b5c1d228f8f08ab9761f3048bde9e12c466

-- 
View it on GitLab: 
https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/cb0f7b5c1d228f8f08ab9761f3048bde9e12c466
You're receiving this email because of your account on gitlab.torproject.org.


_______________________________________________
tor-commits mailing list -- tor-commits@lists.torproject.org
To unsubscribe send an email to tor-commits-le...@lists.torproject.org

Reply via email to