Bug 1490234: Make permissions specific for Windows shared memory handle. r=jld
authorBob Owen <bobowencode@gmail.com>
Tue, 18 Sep 2018 11:50:18 +0100
changeset 437041 e628509972b9950845be5585d16a9f3892e3e48a
parent 437040 2307c43683ba891545d7a6d2c927de4df9be94ca
child 437042 64c64fc5e163e3299803d71606582a058e2cf43b
push id34668
push useraiakab@mozilla.com
push dateWed, 19 Sep 2018 02:19:16 +0000
treeherdermozilla-central@a9e339b3e5d8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjld
bugs1490234
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1490234: Make permissions specific for Windows shared memory handle. r=jld
ipc/chromium/src/base/shared_memory.h
ipc/chromium/src/base/shared_memory_win.cc
--- a/ipc/chromium/src/base/shared_memory.h
+++ b/ipc/chromium/src/base/shared_memory.h
@@ -122,16 +122,19 @@ class SharedMemory {
 #endif
 
  private:
   bool ShareToProcessCommon(ProcessId target_pid,
                             SharedMemoryHandle* new_handle,
                             bool close_self);
 
 #if defined(OS_WIN)
+  // If true indicates this came from an external source so needs extra checks
+  // before being mapped.
+  bool external_section_;
   HANDLE             mapped_file_;
 #elif defined(OS_POSIX)
   int                mapped_file_;
 #endif
   void*              memory_;
   bool               read_only_;
   size_t             max_size_;
 
--- a/ipc/chromium/src/base/shared_memory_win.cc
+++ b/ipc/chromium/src/base/shared_memory_win.cc
@@ -6,32 +6,78 @@
 
 #include "base/shared_memory.h"
 
 #include "base/logging.h"
 #include "base/win_util.h"
 #include "base/string_util.h"
 #include "mozilla/ipc/ProtocolUtils.h"
 
+namespace {
+// NtQuerySection is an internal (but believed to be stable) API and the
+// structures it uses are defined in nt_internals.h.
+// So we have to define them ourselves.
+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);
+
+// 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 =
+    reinterpret_cast<NtQuerySectionType>(
+      ::GetProcAddress(::GetModuleHandle(L"ntdll.dll"), "NtQuerySection"));
+  DCHECK(nt_query_section_func);
+
+  // 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 (unnamed)
+
 namespace base {
 
 SharedMemory::SharedMemory()
-    : mapped_file_(NULL),
+    : external_section_(false),
+      mapped_file_(NULL),
       memory_(NULL),
       read_only_(false),
       max_size_(0) {
 }
 
 SharedMemory::~SharedMemory() {
+  external_section_ = true;
   Close();
 }
 
 bool SharedMemory::SetHandle(SharedMemoryHandle handle, bool read_only) {
   DCHECK(mapped_file_ == NULL);
 
+  external_section_ = true;
   mapped_file_ = handle;
   read_only_ = read_only;
   return true;
 }
 
 // static
 bool SharedMemory::IsHandleValid(const SharedMemoryHandle& handle) {
   return handle != NULL;
@@ -53,18 +99,22 @@ bool SharedMemory::Create(size_t size) {
   max_size_ = size;
   return true;
 }
 
 bool SharedMemory::Map(size_t bytes) {
   if (mapped_file_ == NULL)
     return false;
 
+  if (external_section_ && !IsSectionSafeToMap(mapped_file_)) {
+    return false;
+  }
+
   memory_ = MapViewOfFile(mapped_file_,
-      read_only_ ? FILE_MAP_READ : FILE_MAP_ALL_ACCESS, 0, 0, bytes);
+      read_only_ ? FILE_MAP_READ : FILE_MAP_READ | FILE_MAP_WRITE, 0, 0, bytes);
   if (memory_ != NULL) {
     return true;
   }
   return false;
 }
 
 bool SharedMemory::Unmap() {
   if (memory_ == NULL)
@@ -74,17 +124,17 @@ bool SharedMemory::Unmap() {
   memory_ = NULL;
   return true;
 }
 
 bool SharedMemory::ShareToProcessCommon(ProcessId processId,
                                         SharedMemoryHandle *new_handle,
                                         bool close_self) {
   *new_handle = 0;
-  DWORD access = STANDARD_RIGHTS_REQUIRED | FILE_MAP_READ;
+  DWORD access = FILE_MAP_READ | SECTION_QUERY;
   DWORD options = 0;
   HANDLE mapped_file = mapped_file_;
   HANDLE result;
   if (!read_only_)
     access |= FILE_MAP_WRITE;
   if (close_self) {
     // DUPLICATE_CLOSE_SOURCE causes DuplicateHandle to close mapped_file.
     options = DUPLICATE_CLOSE_SOURCE;