Added protection against mapping image sections between processes.

base/memory/shared_memory_unittest.cc

 // Copyright (c) 2012 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 "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/shared_memory.h"
+#include "base/memory/shared_memory_handle.h"
 #include "base/process/kill.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_info.h"
 #include "base/test/multiprocess_test.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/multiprocess_func_list.h"
 
@@ skipping 545 matching lines @@
 TEST(SharedMemoryTest, MapMinimumAlignment) {
   static const int kDataSize = 8192;
 
   SharedMemory shared_memory;
   ASSERT_TRUE(shared_memory.CreateAndMapAnonymous(kDataSize));
   EXPECT_EQ(0U, reinterpret_cast<uintptr_t>(
       shared_memory.memory()) & (SharedMemory::MAP_MINIMUM_ALIGNMENT - 1));
   shared_memory.Close();
 }
 
+#if defined(OS_WIN)
+TEST(SharedMemoryTest, UnsafeImageSection) {
+  const char* kTestSectionName = "UnsafeImageSection";

[Lei Zhang, 2015/12/16 20:44:10]

const char kFoo[] ?

[forshaw, 2015/12/17 11:38:19]

Done.

+  wchar_t path[MAX_PATH];
+  EXPECT_GT(::GetModuleFileName(nullptr, path, arraysize(path)), 0U);
+
+  // Map the current executable image to save us creating a new PE file on disk.
+  base::win::ScopedHandle file_handle(
+      ::CreateFile(path, GENERIC_READ, 0, nullptr, OPEN_EXISTING, 0, nullptr));
+  EXPECT_TRUE(file_handle.IsValid());
+  base::win::ScopedHandle section_handle(
+      ::CreateFileMappingA(file_handle.Get(), nullptr,
+                           PAGE_READONLY | SEC_IMAGE, 0, 0, kTestSectionName));
+  EXPECT_TRUE(section_handle.IsValid());
+
+  // Check direct opening by name, from handle and duplicated from handle.
+  SharedMemory shared_memory_open;
+  EXPECT_TRUE(shared_memory_open.Open(kTestSectionName, true));
+  EXPECT_FALSE(shared_memory_open.Map(1));
+  EXPECT_EQ(nullptr, shared_memory_open.memory());
+
+  SharedMemory shared_memory_handle_dup(
+      SharedMemoryHandle(section_handle.Get(), ::GetCurrentProcessId()), true,
+      GetCurrentProcess());
+  EXPECT_FALSE(shared_memory_handle_dup.Map(1));
+  EXPECT_EQ(nullptr, shared_memory_handle_dup.memory());
+
+  SharedMemory shared_memory_handle_local(
+      SharedMemoryHandle(section_handle.Take(), ::GetCurrentProcessId()), true);
+  EXPECT_FALSE(shared_memory_handle_local.Map(1));
+  EXPECT_EQ(nullptr, shared_memory_handle_local.memory());
+
+  // Check that a handle without SECTION_QUERY also can't be mapped as it can't

[Lei Zhang, 2015/12/16 20:44:10]

This should not happen with IPC under normal operation, right? I suppose if we
hit this case with this CL landed, we'll know where we lack test coverage.

[forshaw, 2015/12/17 11:38:19]

Yes this shouldn't happen as long as I've caught all the places where section
handles are duplicated. But I think it's valuable to test.

+  // be checked.
+  SharedMemory shared_memory_handle_dummy;
+  SharedMemoryCreateOptions options;
+  options.size = 0x1000;
+  EXPECT_TRUE(shared_memory_handle_dummy.Create(options));
+  HANDLE handle_no_query;
+  EXPECT_TRUE(::DuplicateHandle(
+      ::GetCurrentProcess(), shared_memory_handle_dummy.handle().GetHandle(),
+      ::GetCurrentProcess(), &handle_no_query, FILE_MAP_READ, FALSE, 0));

[Lei Zhang, 2015/12/16 20:44:10]

If you don't need FILE_MAP_READ, you can just call
SharedMemory::DuplicateHandle().

[forshaw, 2015/12/17 11:38:19]

It needs to be explicitly duplicated as SharedMemory::DuplicateHandle will
create a new handle with all the same access rights as the original one. This
needs to explicitly remove the SECTION_QUERY right to do the test.

+  SharedMemory shared_memory_handle_no_query(
+      SharedMemoryHandle(handle_no_query, ::GetCurrentProcessId()), true);
+  EXPECT_FALSE(shared_memory_handle_no_query.Map(1));
+  EXPECT_EQ(nullptr, shared_memory_handle_no_query.memory());
+}
+#endif  // defined(OS_WIN)
+
 // iOS does not allow multiple processes.
 // Android ashmem does not support named shared memory.
 // Mac SharedMemory does not support named shared memory. crbug.com/345734
 #if !defined(OS_IOS) && !defined(OS_ANDROID) && !defined(OS_MACOSX)
 // On POSIX it is especially important we test shmem across processes,
 // not just across threads.  But the test is enabled on all platforms.
 class SharedMemoryProcessTest : public MultiProcessTest {
  public:
   static void CleanUp() {
     SharedMemory memory;
@@ skipping 61 matching lines @@
   memory.Close();
   SharedMemoryProcessTest::CleanUp();
 }
 
 MULTIPROCESS_TEST_MAIN(SharedMemoryTestMain) {
   return SharedMemoryProcessTest::TaskTestMain();
 }
 #endif  // !defined(OS_IOS) && !defined(OS_ANDROID) && !defined(OS_MACOSX)
 
 }  // namespace base