Bug 1258317 - Part 2: Remove unused privilege in mozilla::ipc::TransferHandleTorProcess(). r?gabor draft
authorCervantes Yu <cyu@mozilla.com>
Fri, 06 May 2016 19:01:29 +0800
changeset 367814 c8607fcfa81a324dfc9c44d2efed4b9699d23192
parent 366346 c3f5e6079284a7b7053c41f05d0fe06ff031db03
child 521108 246fb10d9f8c9383b91e6670c62d1c8eaf0bf683
push id18354
push usercyu@mozilla.com
push dateTue, 17 May 2016 10:55:04 +0000
reviewersgabor
bugs1258317
milestone49.0a1
Bug 1258317 - Part 2: Remove unused privilege in mozilla::ipc::TransferHandleTorProcess(). r?gabor Also fix a handle leak in mozilla::ipc::AnnotateProcessInformation(). MozReview-Commit-ID: DuepDytfoD2
ipc/glue/ProtocolUtils.cpp
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -19,25 +19,38 @@
 #if defined(MOZ_SANDBOX) && defined(XP_WIN)
 #define TARGET_SANDBOX_EXPORTS
 #include "mozilla/sandboxTarget.h"
 #endif
 
 #if defined(MOZ_CRASHREPORTER) && defined(XP_WIN)
 #include "aclapi.h"
 #include "sddl.h"
+
+#include "mozilla/TypeTraits.h"
 #endif
 
 using namespace IPC;
 
 using base::GetCurrentProcId;
 using base::ProcessHandle;
 using base::ProcessId;
 
 namespace mozilla {
+
+#if defined(MOZ_CRASHREPORTER) && defined(XP_WIN)
+// Generate RAII classes for LPTSTR and PSECURITY_DESCRIPTOR.
+MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedLPTStr, \
+                                          RemovePointer<LPTSTR>::Type, \
+                                          ::LocalFree)
+MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedPSecurityDescriptor, \
+                                          RemovePointer<PSECURITY_DESCRIPTOR>::Type, \
+                                          ::LocalFree)
+#endif
+
 namespace ipc {
 
 ProtocolCloneContext::ProtocolCloneContext()
   : mNeckoParent(nullptr)
 {}
 
 ProtocolCloneContext::~ProtocolCloneContext()
 {}
@@ -288,18 +301,25 @@ bool DuplicateHandle(HANDLE aSourceHandl
                                                        aTargetHandle,
                                                        aDesiredAccess,
                                                        aOptions)) {
     return true;
   }
 #endif
 
   // Finally, see if we already have access to the process.
-  ScopedProcessHandle targetProcess;
-  if (!base::OpenProcessHandle(aTargetProcessId, &targetProcess.rwget())) {
+  ScopedProcessHandle targetProcess(OpenProcess(PROCESS_DUP_HANDLE,
+                                                FALSE,
+                                                aTargetProcessId));
+  if (!targetProcess) {
+#ifdef MOZ_CRASH_REPORTER
+    CrashReporter::AnnotateCrashReport(
+      NS_LITERAL_CSTRING("IPCTransportFailureReason"),
+      NS_LITERAL_CSTRING("Failed to open target process."));
+#endif
     return false;
   }
 
   return !!::DuplicateHandle(::GetCurrentProcess(), aSourceHandle,
                               targetProcess, aTargetHandle,
                               aDesiredAccess, FALSE, aOptions);
 }
 #endif
@@ -320,17 +340,18 @@ AnnotateSystemError()
       nsPrintfCString("%lld", error));
   }
 }
 
 void
 AnnotateProcessInformation(base::ProcessId aPid)
 {
 #ifdef XP_WIN
-  HANDLE processHandle = OpenProcess(READ_CONTROL|PROCESS_QUERY_INFORMATION, FALSE, aPid);
+  ScopedProcessHandle processHandle(
+    OpenProcess(READ_CONTROL|PROCESS_QUERY_INFORMATION, FALSE, aPid));
   if (!processHandle) {
     CrashReporter::AnnotateCrashReport(
       NS_LITERAL_CSTRING("IPCExtraSystemError"),
       nsPrintfCString("Failed to get information of process %d, error(%d)",
                       aPid,
                       ::GetLastError()));
     return;
   }
@@ -349,61 +370,57 @@ AnnotateProcessInformation(base::Process
     CrashReporter::AnnotateCrashReport(
       NS_LITERAL_CSTRING("IPCExtraSystemError"),
       nsPrintfCString("Process %d is not alive. Exit code: %d",
                       aPid,
                       exitCode));
     return;
   }
 
-  PSECURITY_DESCRIPTOR secDesc = nullptr;
+  ScopedPSecurityDescriptor secDesc(nullptr);
   PSID ownerSid = nullptr;
   DWORD rv = ::GetSecurityInfo(processHandle,
                                SE_KERNEL_OBJECT,
                                OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
                                &ownerSid,
                                nullptr,
                                nullptr,
                                nullptr,
-                               &secDesc);
+                               &secDesc.rwget());
   if (rv != ERROR_SUCCESS) {
     // GetSecurityInfo() failed.
     CrashReporter::AnnotateCrashReport(
       NS_LITERAL_CSTRING("IPCExtraSystemError"),
       nsPrintfCString("Failed to get security information of process %d,"
                       " error(%d)",
                       aPid,
                       rv));
     return;
   }
 
-  LPTSTR ownerSidStr = nullptr;
+  ScopedLPTStr ownerSidStr(nullptr);
   nsString annotation{};
   annotation.AppendLiteral("Owner: ");
-  if (::ConvertSidToStringSid(ownerSid, &ownerSidStr)) {
-    annotation.Append(ownerSidStr);
+  if (::ConvertSidToStringSid(ownerSid, &ownerSidStr.rwget())) {
+    annotation.Append(ownerSidStr.get());
   }
-  ::LocalFree(ownerSidStr);
 
-  LPTSTR secDescStr = nullptr;
+  ScopedLPTStr secDescStr(nullptr);
   annotation.AppendLiteral(", Security Descriptor: ");
   if (::ConvertSecurityDescriptorToStringSecurityDescriptor(secDesc,
                                                             SDDL_REVISION_1,
                                                             DACL_SECURITY_INFORMATION,
-                                                            &secDescStr,
+                                                            &secDescStr.rwget(),
                                                             nullptr)) {
-    annotation.Append(secDescStr);
+    annotation.Append(secDescStr.get());
   }
 
   CrashReporter::AnnotateCrashReport(
     NS_LITERAL_CSTRING("IPCExtraSystemError"),
     NS_ConvertUTF16toUTF8(annotation));
-
-  ::LocalFree(secDescStr);
-  ::LocalFree(secDesc);
 #endif
 }
 #endif
 
 void
 LogMessageForProtocol(const char* aTopLevelProtocol, base::ProcessId aOtherPid,
                       const char* aContextDescription,
                       const char* aMessageDescription,