Validate MessageHeader before using

On validation error, mimic a MessageReceiver returning false. Make that trigger the error state of Connector.

BUG=357885

Review URL: https://codereview.chromium.org/229683005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267588 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 395 insertions, 6 deletions

diff --git a/mojo/common/test/test_support_impl.cc b/mojo/common/test/test_support_impl.cc
index 54c3376..ce9e7ac 100644
--- a/mojo/common/test/test_support_impl.cc
+++ b/mojo/common/test/test_support_impl.cc
@@ -4,10 +4,38 @@
 
 #include "mojo/common/test/test_support_impl.h"
 
+#include <stdlib.h>
+#include <string.h>
+
+#include "base/file_util.h"
+#include "base/files/file_enumerator.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/test/perf_log.h"
 
 namespace mojo {
 namespace test {
+namespace {
+
+base::FilePath ResolveSourceRootRelativePath(const char* relative_path) {
+  base::FilePath path;
+  if (!PathService::Get(base::DIR_SOURCE_ROOT, &path))
+    return base::FilePath();
+
+  std::vector<std::string> components;
+  base::SplitString(relative_path, '/', &components);
+
+  for (size_t i = 0; i < components.size(); ++i) {
+    if (!components[i].empty())
+      path = path.AppendASCII(components[i]);
+  }
+
+  return path;
+}
+
+}  // namespace
 
 TestSupportImpl::TestSupportImpl() {
 }
@@ -21,5 +49,24 @@ void TestSupportImpl::LogPerfResult(const char* test_name,
   base::LogPerfResult(test_name, value, units);
 }
 
+FILE* TestSupportImpl::OpenSourceRootRelativeFile(const char* relative_path) {
+  return base::OpenFile(ResolveSourceRootRelativePath(relative_path), "rb");
+}
+
+char** TestSupportImpl::EnumerateSourceRootRelativeDirectory(
+    const char* relative_path) {
+  std::vector<std::string> names;
+  base::FileEnumerator e(ResolveSourceRootRelativePath(relative_path), false,
+                         base::FileEnumerator::FILES);
+  for (base::FilePath name = e.Next(); !name.empty(); name = e.Next())
+    names.push_back(name.BaseName().AsUTF8Unsafe());
+
+  // |names.size() + 1| for null terminator.
+  char** rv = static_cast<char**>(calloc(names.size() + 1, sizeof(char*)));
+  for (size_t i = 0; i < names.size(); ++i)
+    rv[i] = base::strdup(names[i].c_str());
+  return rv;
+}
+
 }  // namespace test
 }  // namespace mojo
diff --git a/mojo/common/test/test_support_impl.h b/mojo/common/test/test_support_impl.h
index ca3f58a..c752b57 100644
--- a/mojo/common/test/test_support_impl.h
+++ b/mojo/common/test/test_support_impl.h
@@ -19,6 +19,9 @@ class TestSupportImpl : public TestSupport {
   virtual void LogPerfResult(const char* test_name,
                              double value,
                              const char* units) OVERRIDE;
+  virtual FILE* OpenSourceRootRelativeFile(const char* relative_path) OVERRIDE;
+  virtual char** EnumerateSourceRootRelativeDirectory(const char* relative_path)
+      OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestSupportImpl);
diff --git a/mojo/mojo_public.gypi b/mojo/mojo_public.gypi
index 8f124e7..f3a8a1d 100644
--- a/mojo/mojo_public.gypi
+++ b/mojo/mojo_public.gypi
@@ -107,6 +107,7 @@
         'mojo_test_support',
       ],
       'sources': [
+        'public/cpp/test_support/lib/test_support.cc',
         'public/cpp/test_support/lib/test_utils.cc',
         'public/cpp/test_support/test_utils.h',
       ],
@@ -134,6 +135,7 @@
         'public/cpp/bindings/tests/router_unittest.cc',
         'public/cpp/bindings/tests/sample_service_unittest.cc',
         'public/cpp/bindings/tests/type_conversion_unittest.cc',
+        'public/cpp/bindings/tests/validation_unittest.cc',
       ],
     },
     {
@@ -243,6 +245,8 @@
         'public/cpp/bindings/lib/message.cc',
         'public/cpp/bindings/lib/message_builder.cc',
         'public/cpp/bindings/lib/message_builder.h',
+        'public/cpp/bindings/lib/message_header_validator.cc',
+        'public/cpp/bindings/lib/message_header_validator.h',
         'public/cpp/bindings/lib/message_internal.h',
         'public/cpp/bindings/lib/message_queue.cc',
         'public/cpp/bindings/lib/message_queue.h',
diff --git a/mojo/public/c/test_support/test_support.h b/mojo/public/c/test_support/test_support.h
index f854647..2b686b2 100644
--- a/mojo/public/c/test_support/test_support.h
+++ b/mojo/public/c/test_support/test_support.h
@@ -7,6 +7,8 @@
 
 // Note: This header should be compilable as C.
 
+#include <stdio.h>
+
 #include "mojo/public/c/test_support/test_support_export.h"
 
 #ifdef __cplusplus
@@ -18,6 +20,27 @@ MOJO_TEST_SUPPORT_EXPORT void MojoTestSupportLogPerfResult(
     double value,
     const char* units);
 
+// Opens a "/"-delimited file path relative to the source root.
+MOJO_TEST_SUPPORT_EXPORT FILE* MojoTestSupportOpenSourceRootRelativeFile(
+    const char* source_root_relative_path);
+
+// Enumerates a "/"-delimited directory path relative to the source root.
+// Returns only regular files. The return value is a heap-allocated array of
+// heap-allocated strings. Each must be free'd separately.
+//
+// The return value is built like so:
+//
+//   char** rv = (char**) calloc(N + 1, sizeof(char*));
+//   rv[0] = strdup("a");
+//   rv[1] = strdup("b");
+//   rv[2] = strdup("c");
+//   ...
+//   rv[N] = NULL;
+//
+MOJO_TEST_SUPPORT_EXPORT
+char** MojoTestSupportEnumerateSourceRootRelativeDirectory(
+    const char* source_root_relative_path);
+
 #ifdef __cplusplus
 }  // extern "C"
 #endif
diff --git a/mojo/public/cpp/bindings/lib/connector.cc b/mojo/public/cpp/bindings/lib/connector.cc
index 319631c..17f9604 100644
--- a/mojo/public/cpp/bindings/lib/connector.cc
+++ b/mojo/public/cpp/bindings/lib/connector.cc
@@ -114,12 +114,14 @@ void Connector::ReadMore() {
   while (true) {
     MojoResult rv;
 
-    rv = ReadAndDispatchMessage(message_pipe_.get(), incoming_receiver_, NULL);
+    bool receiver_result;
+    rv = ReadAndDispatchMessage(message_pipe_.get(), incoming_receiver_,
+                                &receiver_result);
     if (rv == MOJO_RESULT_SHOULD_WAIT) {
       WaitToReadMore();
       break;
     }
-    if (rv != MOJO_RESULT_OK) {
+    if (rv != MOJO_RESULT_OK || !receiver_result) {
       error_ = true;
       break;
     }
diff --git a/mojo/public/cpp/bindings/lib/message.cc b/mojo/public/cpp/bindings/lib/message.cc
index 8ec188e2..1db071f 100644
--- a/mojo/public/cpp/bindings/lib/message.cc
+++ b/mojo/public/cpp/bindings/lib/message.cc
@@ -9,6 +9,8 @@
 
 #include <algorithm>
 
+#include "mojo/public/cpp/bindings/lib/message_header_validator.h"
+
 namespace mojo {
 
 Message::Message()
@@ -73,9 +75,8 @@ MojoResult ReadAndDispatchMessage(MessagePipeHandle handle,
                       &num_handles,
                       MOJO_READ_MESSAGE_FLAG_NONE);
   if (receiver && rv == MOJO_RESULT_OK) {
-    bool result = receiver->Accept(&message);
-    if (receiver_result)
-      *receiver_result = result;
+    *receiver_result =
+        internal::MessageHeaderValidator(receiver).Accept(&message);
   }
 
   return rv;
diff --git a/mojo/public/cpp/bindings/lib/message_header_validator.cc b/mojo/public/cpp/bindings/lib/message_header_validator.cc
new file mode 100644
index 0000000..e5fa3ef
--- /dev/null
+++ b/mojo/public/cpp/bindings/lib/message_header_validator.cc
@@ -0,0 +1,80 @@
+// 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) {
+  // NOTE: Our goal is to preserve support for future extension of the message
+  // header. If we encounter fields we do not understand, we must ignore them.
+
+  // 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) {
+    if (header->num_bytes != sizeof(internal::MessageHeader))
+      return false;
+  } else if (header->num_fields == 3) {
+    if (header->num_bytes != sizeof(internal::MessageHeaderWithRequestID))
+      return false;
+  } else if (header->num_fields > 3) {
+    if (header->num_bytes < sizeof(internal::MessageHeaderWithRequestID))
+      return false;
+  }
+
+  // Validate flags (allow unknown bits):
+
+  // These flags require a RequestID.
+  if (header->num_fields < 3 &&
+        ((header->flags & internal::kMessageExpectsResponse) ||
+         (header->flags & internal::kMessageIsResponse)))
+    return false;
+
+  // These flags are mutually exclusive.
+  if ((header->flags & internal::kMessageExpectsResponse) &&
+      (header->flags & internal::kMessageIsResponse))
+    return false;
+
+  return true;
+}
+
+}  // namespace
+
+MessageHeaderValidator::MessageHeaderValidator(MessageReceiver* next)
+    : next_(next) {
+  assert(next);
+}
+
+bool MessageHeaderValidator::Accept(Message* message) {
+  // Make sure the message header isn't truncated before we start to read it.
+  if (message->data_num_bytes() < message->header()->num_bytes)
+    return false;
+
+  if (!IsValidMessageHeader(message->header()))
+    return false;
+
+  return next_->Accept(message);
+}
+
+bool MessageHeaderValidator::AcceptWithResponder(Message* message,
+                                                 MessageReceiver* responder) {
+  assert(false);  // Not reached!
+  return false;
+}
+
+}  // namespace internal
+}  // namespace mojo
diff --git a/mojo/public/cpp/bindings/lib/message_header_validator.h b/mojo/public/cpp/bindings/lib/message_header_validator.h
new file mode 100644
index 0000000..d7877fb
--- /dev/null
+++ b/mojo/public/cpp/bindings/lib/message_header_validator.h
@@ -0,0 +1,23 @@
+// 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/message.h"
+
+namespace mojo {
+namespace internal {
+
+class MessageHeaderValidator : public MessageReceiver {
+ public:
+  explicit MessageHeaderValidator(MessageReceiver* next);
+
+  virtual bool Accept(Message* message) MOJO_OVERRIDE;
+  virtual bool AcceptWithResponder(Message* message, MessageReceiver* responder)
+      MOJO_OVERRIDE;
+
+ private:
+  MessageReceiver* next_;
+};
+
+}  // namespace internal
+}  // namespace mojo
diff --git a/mojo/public/cpp/bindings/tests/validation_unittest.cc b/mojo/public/cpp/bindings/tests/validation_unittest.cc
new file mode 100644
index 0000000..1dcbf89
--- /dev/null
+++ b/mojo/public/cpp/bindings/tests/validation_unittest.cc
@@ -0,0 +1,128 @@
+// 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 <stdio.h>
+
+#include <algorithm>
+#include <sstream>
+#include <string>
+#include <vector>
+
+#include "mojo/public/cpp/bindings/lib/message_header_validator.h"
+#include "mojo/public/cpp/test_support/test_support.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mojo {
+namespace test {
+namespace {
+
+std::vector<std::string> GetMatchingTests(const std::vector<std::string>& names,
+                                          const std::string& prefix) {
+  const std::string suffix = ".data";
+  std::vector<std::string> tests;
+  for (size_t i = 0; i < names.size(); ++i) {
+    if (names[i].size() >= suffix.size() &&
+        names[i].substr(0, prefix.size()) == prefix &&
+        names[i].substr(names[i].size() - suffix.size()) == suffix)
+      tests.push_back(names[i].substr(0, names[i].size() - suffix.size()));
+  }
+  return tests;
+}
+
+bool ReadDataFile(const std::string& path, std::vector<uint8_t>* result) {
+  FILE* fp = OpenSourceRootRelativeFile(path.c_str());
+  if (!fp) {
+    ADD_FAILURE() << "File not found: " << path;
+    return false;
+  }
+  for (;;) {
+    unsigned int value;
+    int rv = fscanf(fp, "%x", &value);
+    if (rv != 1)
+      break;
+    result->push_back(static_cast<uint8_t>(value & 0xFF));
+  }
+  bool error = ferror(fp);
+  fclose(fp);
+  return !error;
+}
+
+bool ReadResultFile(const std::string& path, std::string* result) {
+  FILE* fp = OpenSourceRootRelativeFile(path.c_str());
+  if (!fp)
+    return false;
+  fseek(fp, 0, SEEK_END);
+  size_t size = static_cast<size_t>(ftell(fp));
+  fseek(fp, 0, SEEK_SET);
+  result->resize(size);
+  size_t size_read = fread(&result->at(0), 1, size, fp);
+  fclose(fp);
+  if (size != size_read)
+    return false;
+  // Result files are new-line delimited text files. Remove any CRs.
+  result->erase(std::remove(result->begin(), result->end(), '\r'),
+                result->end());
+  return true;
+}
+
+std::string GetPath(const std::string& root, const std::string& suffix) {
+  return "mojo/public/interfaces/bindings/tests/data/" + root + suffix;
+}
+
+void RunValidationTest(const std::string& root, std::string (*func)(Message*)) {
+  std::vector<uint8_t> data;
+  ASSERT_TRUE(ReadDataFile(GetPath(root, ".data"), &data));
+
+  std::string expected;
+  ASSERT_TRUE(ReadResultFile(GetPath(root, ".expected"), &expected));
+
+  Message message;
+  message.AllocUninitializedData(static_cast<uint32_t>(data.size()));
+  memcpy(message.mutable_data(), &data[0], data.size());
+
+  std::string result = func(&message);
+  EXPECT_EQ(expected, result) << "failed test: " << root;
+}
+
+class DummyMessageReceiver : public MessageReceiver {
+ public:
+  virtual bool Accept(Message* message) MOJO_OVERRIDE {
+    return true;  // Any message is OK.
+  }
+  virtual bool AcceptWithResponder(Message* message,
+                                   MessageReceiver* responder) MOJO_OVERRIDE {
+    assert(false);
+    return false;
+  }
+};
+
+std::string DumpMessageHeader(Message* message) {
+  DummyMessageReceiver not_reached_receiver;
+  internal::MessageHeaderValidator validator(&not_reached_receiver);
+  bool rv = validator.Accept(message);
+  if (!rv)
+    return "ERROR\n";
+
+  std::ostringstream os;
+  os << "num_bytes: " << message->header()->num_bytes << "\n"
+     << "num_fields: " << message->header()->num_fields << "\n"
+     << "name: " << message->header()->name << "\n"
+     << "flags: " << message->header()->flags << "\n";
+  return os.str();
+}
+
+TEST(ValidationTest, TestAll) {
+  std::vector<std::string> names =
+      EnumerateSourceRootRelativeDirectory(GetPath("", ""));
+
+  std::vector<std::string> header_tests =
+      GetMatchingTests(names, "validate_header_");
+
+  for (size_t i = 0; i < header_tests.size(); ++i)
+    RunValidationTest(header_tests[i], &DumpMessageHeader);
+}
+
+}  // namespace
+}  // namespace test
+}  // namespace mojo
diff --git a/mojo/public/cpp/test_support/lib/test_support.cc b/mojo/public/cpp/test_support/lib/test_support.cc
new file mode 100644
index 0000000..55a3dcc
--- /dev/null
+++ b/mojo/public/cpp/test_support/lib/test_support.cc
@@ -0,0 +1,26 @@
+// 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/test_support/test_support.h"
+
+#include <stdlib.h>
+
+namespace mojo {
+namespace test {
+
+std::vector<std::string> EnumerateSourceRootRelativeDirectory(
+    const std::string& relative_path) {
+  char** names = MojoTestSupportEnumerateSourceRootRelativeDirectory(
+      relative_path.c_str());
+  std::vector<std::string> results;
+  for (char** ptr = names; *ptr != NULL; ++ptr) {
+    results.push_back(*ptr);
+    free(*ptr);
+  }
+  free(names);
+  return results;
+}
+
+}  // namespace test
+}  // namespace mojo
diff --git a/mojo/public/cpp/test_support/test_support.h b/mojo/public/cpp/test_support/test_support.h
index d0a8eb8..eb4d4be 100644
--- a/mojo/public/cpp/test_support/test_support.h
+++ b/mojo/public/cpp/test_support/test_support.h
@@ -5,6 +5,9 @@
 #ifndef MOJO_PUBLIC_CPP_TEST_SUPPORT_TEST_SUPPORT_H_
 #define MOJO_PUBLIC_CPP_TEST_SUPPORT_TEST_SUPPORT_H_
 
+#include <string>
+#include <vector>
+
 #include "mojo/public/c/test_support/test_support.h"
 
 namespace mojo {
@@ -16,6 +19,15 @@ inline void LogPerfResult(const char* test_name,
   MojoTestSupportLogPerfResult(test_name, value, units);
 }
 
+// Opens text file relative to the source root for reading.
+inline FILE* OpenSourceRootRelativeFile(const std::string& relative_path) {
+  return MojoTestSupportOpenSourceRootRelativeFile(relative_path.c_str());
+}
+
+// Returns the list of regular files in a directory relative to the source root.
+std::vector<std::string> EnumerateSourceRootRelativeDirectory(
+    const std::string& relative_path);
+
 }  // namespace test
 }  // namespace mojo
 
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.data b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.data
new file mode 100644
index 0000000..21e7fbc
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.data
@@ -0,0 +1 @@
+0x00
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.expected b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.expected
new file mode 100644
index 0000000..5df7507
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_too_small.expected
@@ -0,0 +1 @@
+ERROR
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.data b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.data
new file mode 100644
index 0000000..f2ee827
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.data
@@ -0,0 +1,4 @@
+0x00 0x00 0x00 0x00
+0x00 0x00 0x00 0x00
+0x00 0x00 0x00 0x00
+0x00 0x00 0x00 0x00
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.expected b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.expected
new file mode 100644
index 0000000..5df7507
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_bad_zeros.expected
@@ -0,0 +1 @@
+ERROR
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.data b/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.data
new file mode 100644
index 0000000..c37c569
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.data
@@ -0,0 +1,4 @@
+0x10 0x00 0x00 0x00
+0x02 0x00 0x00 0x00
+0x00 0x00 0x00 0x00
+0x00 0x00 0x00 0x00
diff --git a/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.expected b/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.expected
new file mode 100644
index 0000000..c29d2ad
--- /dev/null
+++ b/mojo/public/interfaces/bindings/tests/data/validate_header_good_simple.expected
@@ -0,0 +1,4 @@
+num_bytes: 16
+num_fields: 2
+name: 0
+flags: 0
diff --git a/mojo/public/tests/test_support_private.cc b/mojo/public/tests/test_support_private.cc
index d7d02e8..fa5ead4 100644
--- a/mojo/public/tests/test_support_private.cc
+++ b/mojo/public/tests/test_support_private.cc
@@ -5,8 +5,8 @@
 #include "mojo/public/tests/test_support_private.h"
 
 #include <assert.h>
-#include <stddef.h>
 #include <stdio.h>
+#include <stdlib.h>
 
 static mojo::test::TestSupport* g_test_support = NULL;
 
@@ -21,6 +21,26 @@ void MojoTestSupportLogPerfResult(const char* test_name,
     printf("[no test runner]\t%s\t%g\t%s\n", test_name, value, units);
 }
 
+FILE* MojoTestSupportOpenSourceRootRelativeFile(const char* relative_path) {
+  if (g_test_support)
+    return g_test_support->OpenSourceRootRelativeFile(relative_path);
+  printf("[no test runner]\n");
+  return NULL;
+}
+
+char** MojoTestSupportEnumerateSourceRootRelativeDirectory(
+    const char* relative_path) {
+  if (g_test_support)
+    return g_test_support->EnumerateSourceRootRelativeDirectory(relative_path);
+
+  printf("[no test runner]\n");
+
+  // Return empty list:
+  char** rv = static_cast<char**>(calloc(1, sizeof(char*)));
+  rv[0] = NULL;
+  return rv;
+}
+
 }  // extern "C"
 
 namespace mojo {
diff --git a/mojo/public/tests/test_support_private.h b/mojo/public/tests/test_support_private.h
index 3674fec..29983ca 100644
--- a/mojo/public/tests/test_support_private.h
+++ b/mojo/public/tests/test_support_private.h
@@ -5,6 +5,8 @@
 #ifndef MOJO_PUBLIC_TESTS_TEST_SUPPORT_PRIVATE_H_
 #define MOJO_PUBLIC_TESTS_TEST_SUPPORT_PRIVATE_H_
 
+#include <stdio.h>
+
 #include "mojo/public/c/test_support/test_support.h"
 
 namespace mojo {
@@ -23,6 +25,9 @@ class MOJO_TEST_SUPPORT_EXPORT TestSupport {
   virtual void LogPerfResult(const char* test_name,
                              double value,
                              const char* units) = 0;
+  virtual FILE* OpenSourceRootRelativeFile(const char* relative_path) = 0;
+  virtual char** EnumerateSourceRootRelativeDirectory(
+      const char* relative_path) = 0;
 };
 
 }  // namespace test