[mojo-edk] Better validation of untrusted message data

mojo/edk/system/channel_posix.cc

 // Copyright 2016 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 "mojo/edk/system/channel.h"
 
 #include <errno.h>
 #include <sys/uio.h>
 
 #include <algorithm>
 #include <deque>
+#include <limits>
+#include <memory>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/synchronization/lock.h"
 #include "base/task_runner.h"
 #include "mojo/edk/embedder/platform_channel_utils_posix.h"
@@ skipping 103 matching lines @@
       }
     }
     if (write_error) {
       // Do not synchronously invoke OnError(). Write() may have been called by
       // the delegate and we don't want to re-enter it.
       io_task_runner_->PostTask(FROM_HERE,
                                 base::Bind(&ChannelPosix::OnError, this));
     }
   }
 
-  ScopedPlatformHandleVectorPtr GetReadPlatformHandles(
+  bool GetReadPlatformHandles(
       size_t num_handles,
       const void* extra_header,
-      size_t extra_header_size) override {
+      size_t extra_header_size,
+      ScopedPlatformHandleVectorPtr* handles) override {
+    if (num_handles > std::numeric_limits<uint16_t>::max())
+      return false;
 #if defined(OS_MACOSX) && !defined(OS_IOS)
     // On OSX, we can have mach ports which are located in the extra header
     // section.
     using MachPortsEntry = Channel::Message::MachPortsEntry;
     CHECK(extra_header_size >= num_handles * sizeof(MachPortsEntry));
     size_t num_mach_ports = 0;
     const MachPortsEntry* mach_ports =
         reinterpret_cast<const MachPortsEntry*>(extra_header);
     for (size_t i = 0; i < num_handles; i++) {
       if (mach_ports[i].mach_port != MACH_PORT_NULL)
         num_mach_ports++;
     }
     CHECK(num_mach_ports <= num_handles);
-    if (incoming_platform_handles_.size() + num_mach_ports < num_handles)
-      return nullptr;
+    if (incoming_platform_handles_.size() + num_mach_ports < num_handles) {
+      handles->reset();
+      return true;
+    }
 
-    ScopedPlatformHandleVectorPtr handles(
-        new PlatformHandleVector(num_handles));
+    handles->reset(new PlatformHandleVector(num_handles));
     for (size_t i = 0, mach_port_index = 0; i < num_handles; ++i) {
       if (mach_port_index < num_mach_ports &&
           mach_ports[mach_port_index].index == i) {
-        (*handles)[i] = PlatformHandle(
+        (*handles)->at(i) = PlatformHandle(
             static_cast<mach_port_t>(mach_ports[mach_port_index].mach_port));
-        CHECK((*handles)[i].type == PlatformHandle::Type::MACH);
+        CHECK((*handles)->at(i).type == PlatformHandle::Type::MACH);
         // These are actually just Mach port names until they're resolved from
         // the remote process.
-        (*handles)[i].type = PlatformHandle::Type::MACH_NAME;
+        (*handles)->at(i).type = PlatformHandle::Type::MACH_NAME;
         mach_port_index++;
       } else {
         CHECK(!incoming_platform_handles_.empty());
-        (*handles)[i] = incoming_platform_handles_.front();
+        (*handles)->at(i) = incoming_platform_handles_.front();
         incoming_platform_handles_.pop_front();
       }
     }
 #else
-    if (incoming_platform_handles_.size() < num_handles)
-      return nullptr;
+    if (incoming_platform_handles_.size() < num_handles) {
+      handles->reset();
+      return true;
+    }
 
-    ScopedPlatformHandleVectorPtr handles(
-        new PlatformHandleVector(num_handles));
+    handles->reset(new PlatformHandleVector(num_handles));
     for (size_t i = 0; i < num_handles; ++i) {
-      (*handles)[i] = incoming_platform_handles_.front();
+      (*handles)->at(i) = incoming_platform_handles_.front();
       incoming_platform_handles_.pop_front();
     }
 #endif
 
-    return handles;
+    return true;
   }
 
  private:
   ~ChannelPosix() override {
     DCHECK(!read_watcher_);
     DCHECK(!write_watcher_);
     for (auto handle : incoming_platform_handles_)
       handle.CloseIfNecessary();
   }
 
@@ skipping 193 matching lines @@
           messages.pop_back();
         }
         return true;
       }
     }
 
     return true;
   }
 
 #if defined(OS_MACOSX)
-  void OnControlMessage(Message::Header::MessageType message_type,
+  bool OnControlMessage(Message::Header::MessageType message_type,
                         const void* payload,
                         size_t payload_size,
                         ScopedPlatformHandleVectorPtr handles) override {
     switch (message_type) {
       case Message::Header::MessageType::HANDLES_SENT: {
+        if (payload_size == 0)
+          break;
         MessagePtr message(new Channel::Message(
             payload_size, 0, Message::Header::MessageType::HANDLES_SENT_ACK));
         memcpy(message->mutable_payload(), payload, payload_size);
         Write(std::move(message));
+        return true;
+      }
+
+      case Message::Header::MessageType::HANDLES_SENT_ACK: {
+        size_t num_fds = payload_size / sizeof(int);
+        if (num_fds == 0 || payload_size % sizeof(int) != 0)
+          break;
+
+        const int* fds = reinterpret_cast<const int*>(payload);
+        if (!CloseHandles(fds, num_fds))
+          break;
+        return true;
+      }
+
+      default:
         break;
-      }
-      case Message::Header::MessageType::HANDLES_SENT_ACK: {
-        const int* fds = reinterpret_cast<const int*>(payload);
-        size_t num_fds = payload_size / sizeof(*fds);
-        if (payload_size % sizeof(*fds) != 0 || !CloseHandles(fds, num_fds)) {
-          io_task_runner_->PostTask(FROM_HERE,
-                                    base::Bind(&ChannelPosix::OnError, this));
-        }
-        break;
-      }
-      default:
-        NOTREACHED();
     }
+
+    return false;
   }
 
   // Closes handles referenced by |fds|. Returns false if |num_fds| is 0, or if
   // |fds| does not match a sequence of handles in |handles_to_close_|.
   bool CloseHandles(const int* fds, size_t num_fds) {
     base::AutoLock l(handles_to_close_lock_);
     if (!num_fds)
       return false;
 
     auto start =
@@ skipping 53 matching lines @@
 // static
 scoped_refptr<Channel> Channel::Create(
     Delegate* delegate,
     ScopedPlatformHandle platform_handle,
     scoped_refptr<base::TaskRunner> io_task_runner) {
   return new ChannelPosix(delegate, std::move(platform_handle), io_task_runner);
 }
 
 }  // namespace edk
 }  // namespace mojo