[mojo-edk] Sanity checks on ports message parsing

mojo/edk/system/ports/message.cc

Index: mojo/edk/system/ports/message.cc
diff --git a/mojo/edk/system/ports/message.cc b/mojo/edk/system/ports/message.cc
index 2106c15671aec41b5b067e62afae2413e894b1db..5e9f7bbf91bd38b3cff0bbc4811fe89975c56f3d 100644
--- a/mojo/edk/system/ports/message.cc
+++ b/mojo/edk/system/ports/message.cc
@@ -14,11 +14,13 @@ namespace edk {
 namespace ports {
 
 // static
-void Message::Parse(const void* bytes,
+bool Message::Parse(const void* bytes,
                     size_t num_bytes,
                     size_t* num_header_bytes,
                     size_t* num_payload_bytes,
                     size_t* num_ports_bytes) {
+  if (num_bytes < sizeof(EventHeader))
+    return false;
   const EventHeader* header = static_cast<const EventHeader*>(bytes);
   switch (header->type) {
     case EventType::kUser:
@@ -41,24 +43,33 @@ void Message::Parse(const void* bytes,
       *num_header_bytes = sizeof(EventHeader) + sizeof(MergePortEventData);
       break;
     default:
-      CHECK(false) << "Bad event type";
-      return;
+      return false;
   }
 
   if (header->type == EventType::kUser) {
+    if (num_bytes < sizeof(EventHeader) + sizeof(UserEventData))
+      return false;
     const UserEventData* event_data =
         reinterpret_cast<const UserEventData*>(
             reinterpret_cast<const char*>(header + 1));
+    if (event_data->num_ports > std::numeric_limits<uint16_t>::max())
+      return false;
     *num_header_bytes = sizeof(EventHeader) +
                         sizeof(UserEventData) +
                         event_data->num_ports * sizeof(PortDescriptor);
     *num_ports_bytes = event_data->num_ports * sizeof(PortName);
+    if (num_bytes < *num_header_bytes + *num_ports_bytes)

[Oliver Chang, 2016/05/17 03:45:22]

not a concrete issue, but as a long term goal, would it be feasible to convert
these kinds of integer arithmetic (dealing with offsets and sizes) to use the
safe numerics library (//base/numerics) ?

[Ken Rockot(use gerrit already), 2016/05/17 03:59:01]

Absolutely. Filed http://crbug.com/612374 to do it some time soon. I didn't know
this library existed!

[Oliver Chang, 2016/05/17 04:02:45]

woohoo!

+      return false;
     *num_payload_bytes = num_bytes - *num_header_bytes - *num_ports_bytes;
   } else {
+    if (*num_header_bytes > num_bytes)

[Oliver Chang, 2016/05/17 03:45:22]

Should this be a != comparison instead of >? (based on the DCHECK on line 69,
which can be removed).

[Ken Rockot(use gerrit already), 2016/05/17 03:59:02]

Done

+      return false;
     *num_payload_bytes = 0;
     *num_ports_bytes = 0;
     DCHECK_EQ(num_bytes, *num_header_bytes);
   }
+
+  return true;
 }
 
 Message::Message(size_t num_payload_bytes, size_t num_ports)