fix: guard runtime IPC by owner PID

client/crashpad_client_mac.cc

@@ -19,6 +19,7 @@
 #include <mach/mach.h>
 #include <pthread.h>
 #include <stdint.h>
+#include <unistd.h>
 
 #include <memory>
 #include <tuple>
@@ -407,6 +408,7 @@ class HandlerStarter final : public NotifyServer::DefaultInterface {
       argv.push_back("--wait-for-upload");
     }
 
+    argv.push_back(FormatArgumentInt("client-pid", getpid()));
     argv.push_back(FormatArgumentInt("handshake-fd", server_write_fd.get()));
 
     // When restarting, reset the system default crash handler first. Otherwise,

handler/handler_main.cc

@@ -128,6 +128,8 @@ void Usage(const base::FilePath& me) {
 #if BUILDFLAG(IS_APPLE)
       // clang-format off
 "      --handshake-fd=FD       establish communication with the client over FD\n"
+"      --client-pid=PID\n"
+"                              process ID allowed to send runtime controls\n"
   // clang-format on
 #endif  // BUILDFLAG(IS_APPLE)
 #if BUILDFLAG(IS_WIN)
@@ -245,6 +247,7 @@ struct Options {
 #if BUILDFLAG(IS_APPLE)
   std::string mach_service;
   int handshake_fd;
+  pid_t client_pid;
   bool reset_own_crash_exception_port_to_system_default;
 #elif BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID)
   VMAddress exception_information_address;
@@ -651,6 +654,7 @@ int HandlerMain(int argc,
     kOptionDatabase,
 #if BUILDFLAG(IS_APPLE)
     kOptionHandshakeFD,
+    kOptionClientPID,
 #endif  // BUILDFLAG(IS_APPLE)
 #if BUILDFLAG(IS_WIN)
     kOptionInitialClientData,
@@ -719,6 +723,7 @@ int HandlerMain(int argc,
     {"database", required_argument, nullptr, kOptionDatabase},
 #if BUILDFLAG(IS_APPLE)
     {"handshake-fd", required_argument, nullptr, kOptionHandshakeFD},
+    {"client-pid", required_argument, nullptr, kOptionClientPID},
 #endif  // BUILDFLAG(IS_APPLE)
 #if BUILDFLAG(IS_WIN)
     {"initial-client-data",
@@ -812,6 +817,7 @@ int HandlerMain(int argc,
   Options options = {};
 #if BUILDFLAG(IS_APPLE)
   options.handshake_fd = -1;
+  options.client_pid = -1;
 #endif
   options.identify_client_via_url = true;
 #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_ANDROID)
@@ -862,6 +868,17 @@ int HandlerMain(int argc,
         }
         break;
       }
+      case kOptionClientPID: {
+        int client_pid;
+        if (!StringToNumber(optarg, &client_pid) ||
+            client_pid <= 0) {
+          ToolSupport::UsageHint(
+              me, "--client-pid requires a positive process ID");
+          return ExitFailure();
+        }
+        options.client_pid = client_pid;
+        break;
+      }
       case kOptionMachService: {
         options.mach_service = optarg;
         break;
@@ -1052,6 +1069,10 @@ int HandlerMain(int argc,
         me, "--handshake-fd and --mach-service are incompatible");
     return ExitFailure();
   }
+  if (options.handshake_fd >= 0 && options.client_pid <= 0) {
+    ToolSupport::UsageHint(me, "--client-pid is required with --handshake-fd");
+    return ExitFailure();
+  }
 #elif BUILDFLAG(IS_WIN)
   if (!options.initial_client_data.IsValid() && options.pipe_name.empty()) {
     ToolSupport::UsageHint(me,
@@ -1263,8 +1284,9 @@ int HandlerMain(int argc,
     return ExitFailure();
   }
 
-  ExceptionHandlerServer exception_handler_server(
-      std::move(receive_right), !options.mach_service.empty());
+  ExceptionHandlerServer exception_handler_server(std::move(receive_right),
+                                                  !options.mach_service.empty(),
+                                                  options.client_pid);
   base::AutoReset<ExceptionHandlerServer*> reset_g_exception_handler_server(
       &g_exception_handler_server, &exception_handler_server);
 

handler/linux/exception_handler_server.cc

@@ -243,7 +243,8 @@ ExceptionHandlerServer::ExceptionHandlerServer()
       strategy_decider_(new PtraceStrategyDeciderImpl()),
       delegate_(nullptr),
       pollfd_(),
-      keep_running_(true) {}
+      keep_running_(true),
+      owner_process_id_(-1) {}
 
 ExceptionHandlerServer::~ExceptionHandlerServer() = default;
 
@@ -256,6 +257,23 @@ bool ExceptionHandlerServer::InitializeWithClient(ScopedFileHandle sock,
                                                   bool multiple_clients) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
+  ucred owner_credentials;
+  socklen_t owner_credentials_length = sizeof(owner_credentials);
+  if (getsockopt(sock.get(),
+                 SOL_SOCKET,
+                 SO_PEERCRED,
+                 &owner_credentials,
+                 &owner_credentials_length) != 0) {
+    PLOG(ERROR) << "getsockopt";
+    return false;
+  }
+  if (owner_credentials_length != sizeof(owner_credentials) ||
+      owner_credentials.pid <= 0) {
+    LOG(ERROR) << "invalid owner credentials";
+    return false;
+  }
+  owner_process_id_ = owner_credentials.pid;
+
   pollfd_.reset(epoll_create1(EPOLL_CLOEXEC));
   if (!pollfd_.is_valid()) {
     PLOG(ERROR) << "epoll_create1";
@@ -350,6 +368,27 @@ void ExceptionHandlerServer::HandleEvent(Event* event, uint32_t event_type) {
   return;
 }
 
+static bool RuntimeMessageOriginIsOwner(pid_t owner_process_id,
+                                        const ucred& creds) {
+  if (owner_process_id <= 0) {
+    LOG(WARNING) << "rejecting runtime control message without owner pid";
+    return false;
+  }
+
+  if (creds.pid <= 0) {
+    LOG(WARNING) << "rejecting runtime control message without sender pid";
+    return false;
+  }
+
+  if (creds.pid != owner_process_id) {
+    LOG(WARNING) << "rejecting runtime control message from non-owner pid "
+                 << creds.pid;
+    return false;
+  }
+
+  return true;
+}
+
 bool ExceptionHandlerServer::InstallClientSocket(ScopedFileHandle socket,
                                                  Event::Type type) {
   // The handler may not have permission to set SO_PASSCRED on the socket, but
@@ -433,14 +472,23 @@ bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) {
           event->type == Event::Type::kSharedSocketMessage);
 
     case ExceptionHandlerProtocol::ClientToServerMessage::kTypeAddAttachment:
+      if (!RuntimeMessageOriginIsOwner(owner_process_id_, creds)) {
+        return true;
+      }
       delegate_->AddAttachment(base::FilePath(message.attachment_info.path));
       return true;
 
     case ExceptionHandlerProtocol::ClientToServerMessage::kTypeRemoveAttachment:
+      if (!RuntimeMessageOriginIsOwner(owner_process_id_, creds)) {
+        return true;
+      }
       delegate_->RemoveAttachment(base::FilePath(message.attachment_info.path));
       return true;
 
     case ExceptionHandlerProtocol::ClientToServerMessage::kTypeRequestRetry:
+      if (!RuntimeMessageOriginIsOwner(owner_process_id_, creds)) {
+        return true;
+      }
       delegate_->RequestRetry();
       return true;
   }

handler/linux/exception_handler_server.h

@@ -199,6 +199,7 @@ class ExceptionHandlerServer {
   Delegate* delegate_;
   ScopedFileHandle pollfd_;
   std::atomic<bool> keep_running_;
+  pid_t owner_process_id_;
   InitializationStateDcheck initialized_;
 };
 

handler/mac/exception_handler_server.cc

@@ -37,8 +37,9 @@ namespace {
 // type.
 class ClientToServerMessageServer : public MachMessageServer::Interface {
  public:
-  explicit ClientToServerMessageServer(ExceptionHandlerServer::Delegate* delegate)
-      : delegate_(delegate) {}
+  ClientToServerMessageServer(ExceptionHandlerServer::Delegate* delegate,
+                              pid_t owner_process_id)
+      : delegate_(delegate), owner_process_id_(owner_process_id) {}
 
   ClientToServerMessageServer(const ClientToServerMessageServer&) = delete;
   ClientToServerMessageServer& operator=(const ClientToServerMessageServer&) =
@@ -53,6 +54,10 @@ class ClientToServerMessageServer : public MachMessageServer::Interface {
       return false;
     }
     *destroy_complex_request = true;
+    if (!RuntimeMessageOriginIsOwner(in_header)) {
+      return true;
+    }
+
     switch (message->type) {
       case ClientToServerMessage::kRequestRetry:
         delegate_->RequestRetry();
@@ -80,7 +85,30 @@ class ClientToServerMessageServer : public MachMessageServer::Interface {
   }
 
  private:
+  bool RuntimeMessageOriginIsOwner(const mach_msg_header_t* in_header) const {
+    if (owner_process_id_ <= 0) {
+      // No owner is configured for launchd Mach service handlers.
+      return true;
+    }
+
+    pid_t sender_process_id =
+        AuditPIDFromMachMessageTrailer(MachMessageTrailerFromHeader(in_header));
+    if (sender_process_id <= 0) {
+      LOG(WARNING) << "rejecting runtime control message without sender pid";
+      return false;
+    }
+
+    if (sender_process_id != owner_process_id_) {
+      LOG(WARNING) << "rejecting runtime control message from non-owner pid "
+                   << sender_process_id;
+      return false;
+    }
+
+    return true;
+  }
+
   ExceptionHandlerServer::Delegate* delegate_;  // weak
+  pid_t owner_process_id_;
 };
 
 class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
@@ -89,12 +117,13 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
   ExceptionHandlerServerRun(mach_port_t exception_port,
                             mach_port_t notify_port,
                             bool launchd,
-                            ExceptionHandlerServer::Delegate* delegate)
+                            ExceptionHandlerServer::Delegate* delegate,
+                            pid_t owner_process_id)
       : UniversalMachExcServer::Interface(),
         NotifyServer::DefaultInterface(),
         mach_exc_server_(this),
         notify_server_(this),
-        client_to_server_message_server_(delegate),
+        client_to_server_message_server_(delegate, owner_process_id),
         composite_mach_message_server_(),
         delegate_(delegate),
         exception_port_(exception_port),
@@ -251,9 +280,11 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
 
 ExceptionHandlerServer::ExceptionHandlerServer(
     base::apple::ScopedMachReceiveRight receive_port,
-    bool launchd)
+    bool launchd,
+    pid_t owner_process_id)
     : receive_port_(std::move(receive_port)),
       notify_port_(NewMachPort(MACH_PORT_RIGHT_RECEIVE)),
+      owner_process_id_(owner_process_id),
       launchd_(launchd) {
   CHECK(receive_port_.is_valid());
   CHECK(notify_port_.is_valid());
@@ -263,8 +294,11 @@ ExceptionHandlerServer::~ExceptionHandlerServer() {
 }
 
 void ExceptionHandlerServer::Run(Delegate* delegate) {
-  ExceptionHandlerServerRun run(
-      receive_port_.get(), notify_port_.get(), launchd_, delegate);
+  ExceptionHandlerServerRun run(receive_port_.get(),
+                                notify_port_.get(),
+                                launchd_,
+                                delegate,
+                                owner_process_id_);
   run.Run();
 }
 

handler/mac/exception_handler_server.h

@@ -16,6 +16,7 @@
 #define CRASHPAD_HANDLER_MAC_EXCEPTION_HANDLER_SERVER_H_
 
 #include <mach/mach.h>
+#include <sys/types.h>
 
 #include "base/apple/scoped_mach_port.h"
 #include "base/files/file_path.h"
@@ -54,8 +55,11 @@ class ExceptionHandlerServer {
   //!     launchd. \a receive_port is not monitored for no-senders
   //!     notifications, and instead, Stop() must be called to provide a “quit”
   //!     signal.
+  //! \param[in] owner_process_id The process ID allowed to send runtime
+  //!     control messages.
   ExceptionHandlerServer(base::apple::ScopedMachReceiveRight receive_port,
-                         bool launchd);
+                         bool launchd,
+                         pid_t owner_process_id = -1);
 
   ExceptionHandlerServer(const ExceptionHandlerServer&) = delete;
   ExceptionHandlerServer& operator=(const ExceptionHandlerServer&) = delete;
@@ -94,6 +98,7 @@ class ExceptionHandlerServer {
  private:
   base::apple::ScopedMachReceiveRight receive_port_;
   base::apple::ScopedMachReceiveRight notify_port_;
+  pid_t owner_process_id_;
   bool launchd_;
 };