[mojo-edk] Better validation of untrusted message data

mojo/edk/system/channel_win.cc

Index: mojo/edk/system/channel_win.cc
diff --git a/mojo/edk/system/channel_win.cc b/mojo/edk/system/channel_win.cc
index 816c849411938cb91f1ce71a13b69cb88744c463..96a59cf3a7b5013569f9bdeb30ff7a9604537f09 100644
--- a/mojo/edk/system/channel_win.cc
+++ b/mojo/edk/system/channel_win.cc
@@ -114,18 +114,18 @@ class ChannelWin : public Channel,
     }
   }
 
-  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 {
     size_t handles_size = sizeof(PlatformHandle) * num_handles;

[Oliver Chang, 2016/05/16 17:11:34]

|num_handles| here is a size_t, which means that it's possible for this
calculation to overflow (similar in channel_posix.cc).

It seems that mojo::edk::Channel::Message::Header::num_handles is uint16_t, but
could we still get a sanity check for num_handles <=
numeric_limits<uint16_t>::max()?

[Ken Rockot(use gerrit already), 2016/05/16 17:15:32]

Done

     if (handles_size > extra_header_size)
-      return nullptr;
-
-    ScopedPlatformHandleVectorPtr handles(
-        new PlatformHandleVector(num_handles));
-    memcpy(handles->data(), extra_header, handles_size);
-    return handles;
+      return false;
+    DCHECK(extra_header);
+    handles->reset(new PlatformHandleVector(num_handles));
+    memcpy((*handles)->data(), extra_header, handles_size);
+    return true;
   }
 
  private: