[mojo-edk] Fix unchecked header sizes channel messages

mojo/edk/system/channel.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 <string.h>
 
 #include <algorithm>
 #include <limits>
@@ skipping 119 matching lines @@
   if (data_num_bytes < sizeof(Header))
     return nullptr;
 
   const Header* header = reinterpret_cast<const Header*>(data);
   if (header->num_bytes != data_num_bytes) {
     DLOG(ERROR) << "Decoding invalid message: " << header->num_bytes
                 << " != " << data_num_bytes;
     return nullptr;
   }
 
-  if (header->num_bytes < header->num_header_bytes) {
+  if (header->num_bytes < header->num_header_bytes ||
+      header->num_header_bytes < sizeof(Header)) {

[Ken Rockot(use gerrit already), 2016/06/23 00:26:37]

I made this condition explicit here, but we already know the following things
are true already:

data_num_bytes >= sizeof(Header)
header->num_bytes == data_num_bytes
header->num_header_bytes >= header->num_bytes

How is it not guaranteed then that header->num_header_bytes >= sizeof(Header)?

[Ken Rockot(use gerrit already), 2016/06/23 00:30:35]

Wait, nevermind. I did backwards things in my head. We only know that num_bytes
>= num_header_bytes, which is uninteresting for this bug. :)

[Oliver Chang, 2016/06/23 00:31:17]

yep :) I missed this during the first review too

     DLOG(ERROR) << "Decoding invalid message: " << header->num_bytes << " < "
                 << header->num_header_bytes;
     return nullptr;
   }
 
   uint32_t extra_header_size = header->num_header_bytes - sizeof(Header);
 #if defined(OS_WIN)
   uint32_t max_handles = extra_header_size / sizeof(HandleEntry);
 #elif defined(OS_MACOSX) && !defined(OS_IOS)
+  if (extra_header_size < sizeof(MachPortsExtraHeader)) {
+    DLOG(ERROR) << "Decoding invalid message: " << extra_header_size << " < "
+                << sizeof(MachPortsExtraHeader);
+    return nullptr;
+  }
   uint32_t max_handles = (extra_header_size - sizeof(MachPortsExtraHeader)) /

[Oliver Chang, 2016/06/23 00:31:17]

not sure if it's worth sanity checking max_handles, but up to you if it makes
sense to.

[Ken Rockot(use gerrit already), 2016/06/23 00:42:51]

oh right... done!

       sizeof(MachPortsEntry);
 #endif
   if (header->num_handles > max_handles) {
     DLOG(ERROR) << "Decoding invalid message:" << header->num_handles
                 << " > " << max_handles;
     return nullptr;
   }
 
   MessagePtr message(new Message(data_num_bytes - header->num_header_bytes,
                                  max_handles));
@@ skipping 407 matching lines @@
 
 bool Channel::OnControlMessage(Message::Header::MessageType message_type,
                                const void* payload,
                                size_t payload_size,
                                ScopedPlatformHandleVectorPtr handles) {
   return false;
 }
 
 }  // namespace edk
 }  // namespace mojo