win: Lower integrity level of connection pipe

util/win/exception_handler_server.cc

Index: util/win/exception_handler_server.cc
diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc
index ce5687d74b7ba6f853708f68acf5e4f25390f5b0..46b0e00d0f2efd3418485b029e39fc4d9cc690a4 100644
--- a/util/win/exception_handler_server.cc
+++ b/util/win/exception_handler_server.cc
@@ -14,6 +14,7 @@
 
 #include "util/win/exception_handler_server.h"
 
+#include <sddl.h>
 #include <string.h>
 
 #include "base/logging.h"
@@ -30,6 +31,7 @@
 #include "util/win/get_function.h"
 #include "util/win/handle.h"
 #include "util/win/registration_protocol_win.h"
+#include "util/win/scoped_local_free.h"
 #include "util/win/xp_compat.h"
 
 namespace crashpad {
@@ -44,19 +46,49 @@ const size_t kPipeInstances = 2;
 //
 // If first_instance is true, the named pipe instance will be created with
 // FILE_FLAG_FIRST_PIPE_INSTANCE. This ensures that the the pipe name is not
-// already in use when created.
+// already in use when created. The first instance will be created with an
+// untrusted integrity SACL so instances of this pipe can be connected to by
+// processes of any integrity level.
 HANDLE CreateNamedPipeInstance(const std::wstring& pipe_name,
                                bool first_instance) {
-  return CreateNamedPipe(pipe_name.c_str(),
-                         PIPE_ACCESS_DUPLEX |
-                             (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE
-                                             : 0),
-                         PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
-                         kPipeInstances,
-                         512,
-                         512,
-                         0,
-                         nullptr);
+  SECURITY_ATTRIBUTES security_attributes = {0};

[Mark Mentovai, 2015/11/06 14:55:48]

Can you defer the = {} until you’re sure we’re going to use it, below on line
76?

[scottmg, 2015/11/06 18:03:30]

Done.

+  SECURITY_ATTRIBUTES* security_attributes_pointer = nullptr;
+  ScopedLocalFree scoped_sec_desc;
+
+  if (first_instance) {
+    // Pre-Vista does not have integrity levels.
+    const DWORD version = GetVersion();
+    const DWORD major_version = LOBYTE(LOWORD(version));
+    const bool is_vista_or_later = major_version >= 6;
+    if (is_vista_or_later) {
+      // Mandatory Label, no ACE flags, no ObjectType, integrity level
+      // untrusted.
+      const wchar_t kSddl[] = L"S:(ML;;;;;S-1-16-0)";
+
+      PSECURITY_DESCRIPTOR sec_desc = nullptr;

[Mark Mentovai, 2015/11/06 14:55:48]

No need to initialize.

[scottmg, 2015/11/06 18:03:30]

Done.

+      PCHECK(ConvertStringSecurityDescriptorToSecurityDescriptor(

[Mark Mentovai, 2015/11/06 14:55:48]

Do we need to add -ladvapi32.lib or do we get that by default?

[scottmg, 2015/11/06 18:03:30]

Done.

+          kSddl, SDDL_REVISION_1, &sec_desc, nullptr))
+          << "ConvertStringSecurityDescriptorToSecurityDescriptor";
+
+      // Take ownership of the allocated SECURITY_DESCRIPTOR.
+      scoped_sec_desc.reset(sec_desc);
+
+      security_attributes.nLength = sizeof(SECURITY_ATTRIBUTES);
+      security_attributes.lpSecurityDescriptor = sec_desc;
+      security_attributes.bInheritHandle = FALSE;
+      security_attributes_pointer = &security_attributes;
+    }
+  }
+
+  return CreateNamedPipe(
+      pipe_name.c_str(),
+      PIPE_ACCESS_DUPLEX | (first_instance ? FILE_FLAG_FIRST_PIPE_INSTANCE : 0),
+      PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE | PIPE_WAIT,
+      kPipeInstances,
+      512,
+      512,
+      0,
+      security_attributes_pointer);
 }
 
 decltype(GetNamedPipeClientProcessId)* GetNamedPipeClientProcessIdFunction() {
@@ -320,7 +352,7 @@ void ExceptionHandlerServer::Run(Delegate* delegate) {
     if (first_pipe_instance_.is_valid()) {
       pipe = first_pipe_instance_.release();
     } else {
-      pipe = CreateNamedPipeInstance(pipe_name_, false);
+      pipe = CreateNamedPipeInstance(pipe_name_, i == 0);
       PCHECK(pipe != INVALID_HANDLE_VALUE) << "CreateNamedPipe";
     }