Do not double-unref send rights when using POLICY_SUBSTITUE_PORT.

sandbox/mac/launchd_interception_server.cc

 // Copyright 2014 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/mac/launchd_interception_server.h"
 
 #include <bsm/libbsm.h>
 #include <servers/bootstrap.h>
 
 #include "base/logging.h"
@@ skipping 58 matching lines @@
   }
   reply_buffer_.reset(buffer, kBufferSize);
 
   // Allocate the dummy sandbox port.
   if ((kr = mach_port_allocate(task, MACH_PORT_RIGHT_RECEIVE, &port)) !=
           KERN_SUCCESS) {
     MACH_LOG(ERROR, kr) << "Failed to allocate dummy sandbox port.";
     return false;
   }
   sandbox_port_.reset(port);
+  if ((kr = mach_port_insert_right(task, sandbox_port_, sandbox_port_,
+          MACH_MSG_TYPE_MAKE_SEND) != KERN_SUCCESS)) {
+    MACH_LOG(ERROR, kr) << "Failed to allocate dummy sandbox port send right.";
+    return false;
+  }
+  sandbox_send_port_.reset(sandbox_port_);
 
   // Set up the dispatch queue to service the bootstrap port.
   // TODO(rsesek): Specify DISPATCH_QUEUE_SERIAL, in the 10.7 SDK. NULL means
   // the same thing but is not symbolically clear.
   server_queue_ = dispatch_queue_create(
       "org.chromium.sandbox.LaunchdInterceptionServer", NULL);
   server_source_ = dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_RECV,
       server_port_.get(), 0, server_queue_);
   dispatch_source_set_event_handler(server_source_, ^{ ReceiveMessage(); });
   dispatch_resume(server_source_);
@@ skipping 120 matching lines @@
     // or the one specified in the policy.
     VLOG(1) << "Intercepting look_up2 with a sandboxed service port: "
             << request_service_name;
 
     mach_port_t result_port;
     if (rule.result == POLICY_DENY_DUMMY_PORT)
       result_port = sandbox_port_.get();
     else
       result_port = rule.substitute_port;
 
-    // Grant an additional send right on the result_port so that it can be
-    // sent to the sandboxed child process.
-    kern_return_t kr = mach_port_insert_right(mach_task_self(),
-        result_port, result_port, MACH_MSG_TYPE_MAKE_SEND);
-    if (kr != KERN_SUCCESS) {
-      MACH_LOG(ERROR, kr) << "Unable to insert right on result_port.";
-    }
-
     compat_shim_.look_up2_fill_reply(reply, result_port);
-    SendReply(reply);
+    // If the message was sent successfully, clear the result_port out of the
+    // message so that it is not destroyed at the end of ReceiveMessage. The
+    // above-inserted right has been moved out of the process, and destroying
+    // the message will unref yet another right.
+    if (SendReply(reply))
+      compat_shim_.look_up2_fill_reply(reply, MACH_PORT_NULL);
   } else {
     NOTREACHED();
   }
 }
 
 void LaunchdInterceptionServer::HandleSwapInteger(mach_msg_header_t* request,
                                                   mach_msg_header_t* reply,
                                                   pid_t sender_pid) {
   // Only allow getting information out of launchd. Do not allow setting
   // values. Two commonly observed values that are retrieved are
   // VPROC_GSK_MGR_PID and VPROC_GSK_TRANSACTIONS_ENABLED.
   if (compat_shim_.swap_integer_is_get_only(request)) {
     VLOG(2) << "Forwarding vproc swap_integer message.";
     ForwardMessage(request, reply);
   } else {
     VLOG(2) << "Rejecting non-read-only swap_integer message.";
     RejectMessage(request, reply, BOOTSTRAP_NOT_PRIVILEGED);
   }
 }
 
-void LaunchdInterceptionServer::SendReply(mach_msg_header_t* reply) {
+bool LaunchdInterceptionServer::SendReply(mach_msg_header_t* reply) {
   kern_return_t kr = mach_msg(reply, MACH_SEND_MSG, reply->msgh_size, 0,
       MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
-  if (kr != KERN_SUCCESS) {
-    MACH_LOG(ERROR, kr) << "Unable to send intercepted reply message.";
-  }
+  MACH_LOG_IF(ERROR, kr != KERN_SUCCESS, kr)
+      << "Unable to send intercepted reply message.";
+  return kr == KERN_SUCCESS;
 }
 
 void LaunchdInterceptionServer::ForwardMessage(mach_msg_header_t* request,
                                                mach_msg_header_t* reply) {
   request->msgh_local_port = request->msgh_remote_port;
   request->msgh_remote_port = sandbox_->real_bootstrap_port();
   // Preserve the msgh_bits that do not deal with the local and remote ports.
   request->msgh_bits = (request->msgh_bits & ~MACH_MSGH_BITS_PORTS_MASK) |
       MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, MACH_MSG_TYPE_MOVE_SEND_ONCE);
   kern_return_t kr = mach_msg_send(request);
@@ skipping 10 matching lines @@
   mig_reply_error_t* error_reply = reinterpret_cast<mig_reply_error_t*>(reply);
   error_reply->Head.msgh_size = sizeof(mig_reply_error_t);
   error_reply->Head.msgh_bits =
       MACH_MSGH_BITS_REMOTE(MACH_MSG_TYPE_MOVE_SEND_ONCE);
   error_reply->NDR = NDR_record;
   error_reply->RetCode = error_code;
   SendReply(&error_reply->Head);
 }
 
 }  // namespace sandbox