Validate MessageHeader before using

mojo/public/cpp/bindings/lib/message_header_validator.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 "mojo/public/cpp/bindings/lib/message_header_validator.h"
+
+#include "mojo/public/cpp/bindings/lib/bindings_serialization.h"
+
+namespace mojo {
+namespace internal {
+namespace {
+
+bool IsValidMessageHeader(const internal::MessageHeader* header) {
+  // Validate num_bytes
+  if (header->num_bytes < sizeof(internal::MessageHeader))
+    return false;
+  if (internal::Align(header->num_bytes) != header->num_bytes)
+    return false;
+
+  // Validate num_fields
+  if (header->num_fields < 2)
+    return false;
+  if (header->num_fields == 2 &&

[viettrungluu, 2014/04/25 21:55:59]

Probably this should be structured as:

if (header->num_fields == 2) {
  if (... != ...)
    return false;
} else if (header->num_fields == 3) {
  ...
} else {  // |header->num_fields > 3|.
  if (header->num_bytes < sizeof(internal::MessageHeaderWithRequestID))
    return false;
}

[darin (slow to review), 2014/04/29 06:31:45]

Done.

+      header->num_bytes != sizeof(internal::MessageHeader))
+    return false;
+  if (header->num_fields == 3 &&
+      header->num_bytes != sizeof(internal::MessageHeaderWithRequestID))
+    return false;
+
+  // Validate flags
+  if (header->flags &

[viettrungluu, 2014/04/25 21:55:59]

Do we want to disallow future flags?

[darin (slow to review), 2014/04/29 06:31:45]

No, I don't think we do. Revising code.

+          ~(internal::kMessageExpectsResponse | internal::kMessageIsResponse))
+    return false;
+  if ((header->flags & internal::kMessageExpectsResponse) &&
+          header->num_fields < 3)
+    return false;
+  if ((header->flags & internal::kMessageIsResponse) &&
+          header->num_fields < 3)
+    return false;
+

[viettrungluu, 2014/04/25 21:55:59]

Also check that "expects response" and "is response" aren't both set?

[darin (slow to review), 2014/04/29 06:31:45]

Done.

+  return true;
+}
+
+}  // namespace
+
+MessageHeaderValidator::MessageHeaderValidator(MessageReceiver* next)
+    : next_(next) {
+  assert(next);
+}
+
+bool MessageHeaderValidator::Accept(Message* message) {
+  if (!IsValidMessageHeader(message->header()))
+    return false;  // Simulate an unhappy receiver.
+
+  return next_->Accept(message);
+}
+
+bool MessageHeaderValidator::AcceptWithResponder(Message* message,
+                                                 MessageReceiver* responder) {
+  assert(false);  // Not reached!
+  return false;
+}
+
+}  // namespace internal
+}  // namespace mojo