Added protection against mapping image sections between processes.

base/memory/shared_memory_win.cc

Index: base/memory/shared_memory_win.cc
diff --git a/base/memory/shared_memory_win.cc b/base/memory/shared_memory_win.cc
index fd8d2fdd0c8a00bf91b54b8d5bee6606770e1542..0079433c8d900c63b8f7aa3dc3ef099a41973ff8 100644
--- a/base/memory/shared_memory_win.cc
+++ b/base/memory/shared_memory_win.cc
@@ -14,6 +14,23 @@
 
 namespace {
 
+typedef enum _SECTION_INFORMATION_CLASS {
+  SectionBasicInformation,
+} SECTION_INFORMATION_CLASS;
+
+typedef struct _SECTION_BASIC_INFORMATION {
+  PVOID BaseAddress;
+  ULONG Attributes;
+  LARGE_INTEGER Size;
+} SECTION_BASIC_INFORMATION, *PSECTION_BASIC_INFORMATION;
+
+typedef ULONG(__stdcall* NtQuerySectionType)(
+    HANDLE SectionHandle,
+    SECTION_INFORMATION_CLASS SectionInformationClass,
+    PVOID SectionInformation,
+    ULONG SectionInformationLength,
+    PULONG ResultLength);
+
 // Returns the length of the memory section starting at the supplied address.
 size_t GetMemorySectionSize(void* address) {
   MEMORY_BASIC_INFORMATION memory_info;
@@ -23,6 +40,26 @@ size_t GetMemorySectionSize(void* address) {
          static_cast<char*>(memory_info.AllocationBase));
 }
 
+// Checks if the section object is safe to map. At the moment this just means
+// it's not an image section.
+bool IsSectionSafeToMap(HANDLE handle) {
+  static NtQuerySectionType nt_query_section_func;
+  if (!nt_query_section_func) {
+    nt_query_section_func = reinterpret_cast<NtQuerySectionType>(
+        ::GetProcAddress(::GetModuleHandle(L"ntdll.dll"), "NtQuerySection"));
+    DCHECK(nt_query_section_func != nullptr);

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

omit: != nullptr

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

Done.

+  }
+
+  // The handle must have SECTION_QUERY access for this to succeed.
+  SECTION_BASIC_INFORMATION basic_information = {};
+  ULONG status =
+      nt_query_section_func(handle, SectionBasicInformation, &basic_information,
+                            sizeof(basic_information), nullptr);
+  if (status)
+    return false;
+  return (basic_information.Attributes & SEC_IMAGE) != SEC_IMAGE;
+}
+
 }  // namespace.
 
 namespace base {
@@ -35,24 +72,25 @@ SharedMemoryCreateOptions::SharedMemoryCreateOptions()
       share_read_only(false) {}
 
 SharedMemory::SharedMemory()
-    : mapped_file_(NULL),
+    : external_section_(false),
+      mapped_file_(NULL),
       mapped_size_(0),
       memory_(NULL),
       read_only_(false),
-      requested_size_(0) {
-}
+      requested_size_(0) {}
 
 SharedMemory::SharedMemory(const std::wstring& name)
-    : name_(name),
+    : external_section_(false),
+      name_(name),
       mapped_file_(NULL),
       mapped_size_(0),
       memory_(NULL),
       read_only_(false),
-      requested_size_(0) {
-}
+      requested_size_(0) {}
 
 SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
-    : mapped_file_(handle.GetHandle()),
+    : external_section_(true),
+      mapped_file_(handle.GetHandle()),
       mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
@@ -63,14 +101,16 @@ SharedMemory::SharedMemory(const SharedMemoryHandle& handle, bool read_only)
 SharedMemory::SharedMemory(const SharedMemoryHandle& handle,
                            bool read_only,
                            ProcessHandle process)
-    : mapped_file_(NULL),
+    : external_section_(true),
+      mapped_file_(NULL),
       mapped_size_(0),
       memory_(NULL),
       read_only_(read_only),
       requested_size_(0) {
-  ::DuplicateHandle(
-      process, handle.GetHandle(), GetCurrentProcess(), &mapped_file_,
-      read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE, FALSE, 0);
+  DWORD access = FILE_MAP_READ | SECTION_QUERY;
+  ::DuplicateHandle(process, handle.GetHandle(), GetCurrentProcess(),
+                    &mapped_file_,
+                    read_only_ ? access : access | FILE_MAP_WRITE, FALSE, 0);
 }
 
 SharedMemory::~SharedMemory() {
@@ -170,6 +210,7 @@ bool SharedMemory::Create(const SharedMemoryCreateOptions& options) {
     // If the file already existed, set requested_size_ to 0 to show that
     // we don't know the size.
     requested_size_ = 0;
+    external_section_ = true;
     if (!options.open_existing_deprecated) {
       Close();
       return false;
@@ -186,17 +227,20 @@ bool SharedMemory::Delete(const std::string& name) {
 
 bool SharedMemory::Open(const std::string& name, bool read_only) {
   DCHECK(!mapped_file_);
-
+  DWORD access = FILE_MAP_READ | SECTION_QUERY;
+  if (!read_only)
+    access |= FILE_MAP_WRITE;
   name_ = ASCIIToUTF16(name);
   read_only_ = read_only;
-  mapped_file_ = OpenFileMapping(
-      read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE,
-      false, name_.empty() ? NULL : name_.c_str());
-  if (mapped_file_ != NULL) {
-    // Note: size_ is not set in this case.
-    return true;
-  }
-  return false;
+  mapped_file_ =
+      OpenFileMapping(access, false, name_.empty() ? nullptr : name_.c_str());
+  if (mapped_file_ == nullptr)

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

if (!mapped_file_)

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

Done.

+    return false;
+  // If a name specified assume it's an external section.
+  if (!name_.empty())
+    external_section_ = true;
+  // Note: size_ is not set in this case.
+  return true;
 }
 
 bool SharedMemory::MapAt(off_t offset, size_t bytes) {
@@ -209,6 +253,9 @@ bool SharedMemory::MapAt(off_t offset, size_t bytes) {
   if (memory_)
     return false;
 
+  if (external_section_ && !IsSectionSafeToMap(mapped_file_))
+    return false;
+
   memory_ = MapViewOfFile(mapped_file_,
                           read_only_ ? FILE_MAP_READ : FILE_MAP_READ |
                               FILE_MAP_WRITE,
@@ -238,7 +285,7 @@ bool SharedMemory::ShareToProcessCommon(ProcessHandle process,
                                         bool close_self,
                                         ShareMode share_mode) {
   *new_handle = SharedMemoryHandle();
-  DWORD access = FILE_MAP_READ;
+  DWORD access = FILE_MAP_READ | SECTION_QUERY;
   DWORD options = 0;
   HANDLE mapped_file = mapped_file_;
   HANDLE result;