Bug 1534196: Correct return values from failed brokering. r=jmathies, a=pascalc
authorBob Owen <bobowencode@gmail.com>
Mon, 08 Apr 2019 08:54:27 +0100
changeset 526086 5929b22e9c9a197dfa214962cef54172ab98dadc
parent 526085 20ce903bc29aba3e18550fb1c7e8833a7432c445
child 526087 b4fc373023dfb000596ab54f133200bb9ee4eff0
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjmathies, pascalc
bugs1534196
milestone67.0
Bug 1534196: Correct return values from failed brokering. r=jmathies, a=pascalc
security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
security/sandbox/chromium/sandbox/win/src/named_pipe_policy.cc
security/sandbox/chromium/sandbox/win/src/process_thread_policy.cc
security/sandbox/chromium/sandbox/win/src/registry_policy.cc
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
@@ -251,16 +251,17 @@ bool FileSystemPolicy::CreateFileAction(
                                         uint32_t desired_access,
                                         uint32_t file_attributes,
                                         uint32_t share_access,
                                         uint32_t create_disposition,
                                         uint32_t create_options,
                                         HANDLE* handle,
                                         NTSTATUS* nt_status,
                                         ULONG_PTR* io_information) {
+  *handle = nullptr;
   // The only action supported is ASK_BROKER which means create the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
     return false;
   }
   IO_STATUS_BLOCK io_block = {};
   UNICODE_STRING uni_name = {};
@@ -283,21 +284,22 @@ bool FileSystemPolicy::OpenFileAction(Ev
                                       const base::string16& file,
                                       uint32_t attributes,
                                       uint32_t desired_access,
                                       uint32_t share_access,
                                       uint32_t open_options,
                                       HANDLE* handle,
                                       NTSTATUS* nt_status,
                                       ULONG_PTR* io_information) {
+  *handle = nullptr;
   // The only action supported is ASK_BROKER which means open the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
   // An NtOpen is equivalent to an NtCreate with FileAttributes = 0 and
   // CreateDisposition = FILE_OPEN.
   IO_STATUS_BLOCK io_block = {};
   UNICODE_STRING uni_name = {};
   OBJECT_ATTRIBUTES obj_attributes = {};
   SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS();
 
@@ -318,17 +320,17 @@ bool FileSystemPolicy::QueryAttributesFi
     const base::string16& file,
     uint32_t attributes,
     FILE_BASIC_INFORMATION* file_info,
     NTSTATUS* nt_status) {
   // The only action supported is ASK_BROKER which means query the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
 
   NtQueryAttributesFileFunction NtQueryAttributesFile = NULL;
   ResolveNTFunctionPtr("NtQueryAttributesFile", &NtQueryAttributesFile);
 
   UNICODE_STRING uni_name = {0};
   OBJECT_ATTRIBUTES obj_attributes = {0};
   SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS();
@@ -346,17 +348,17 @@ bool FileSystemPolicy::QueryFullAttribut
     const base::string16& file,
     uint32_t attributes,
     FILE_NETWORK_OPEN_INFORMATION* file_info,
     NTSTATUS* nt_status) {
   // The only action supported is ASK_BROKER which means query the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
 
   NtQueryFullAttributesFileFunction NtQueryFullAttributesFile = NULL;
   ResolveNTFunctionPtr("NtQueryFullAttributesFile", &NtQueryFullAttributesFile);
 
   UNICODE_STRING uni_name = {0};
   OBJECT_ATTRIBUTES obj_attributes = {0};
   SECURITY_QUALITY_OF_SERVICE security_qos = GetAnonymousQOS();
@@ -375,28 +377,28 @@ bool FileSystemPolicy::SetInformationFil
                                                 uint32_t length,
                                                 uint32_t info_class,
                                                 IO_STATUS_BLOCK* io_block,
                                                 NTSTATUS* nt_status) {
   // The only action supported is ASK_BROKER which means open the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
 
   NtSetInformationFileFunction NtSetInformationFile = NULL;
   ResolveNTFunctionPtr("NtSetInformationFile", &NtSetInformationFile);
 
   HANDLE local_handle = NULL;
   if (!::DuplicateHandle(client_info.process, target_file_handle,
                          ::GetCurrentProcess(), &local_handle, 0, FALSE,
                          DUPLICATE_SAME_ACCESS)) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
 
   base::win::ScopedHandle handle(local_handle);
 
   FILE_INFORMATION_CLASS file_info_class =
       static_cast<FILE_INFORMATION_CLASS>(info_class);
   *nt_status = NtSetInformationFile(local_handle, io_block, file_info, length,
                                     file_info_class);
--- a/security/sandbox/chromium/sandbox/win/src/named_pipe_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/named_pipe_policy.cc
@@ -62,16 +62,17 @@ DWORD NamedPipePolicy::CreateNamedPipeAc
                                              const ClientInfo& client_info,
                                              const base::string16 &name,
                                              DWORD open_mode, DWORD pipe_mode,
                                              DWORD max_instances,
                                              DWORD out_buffer_size,
                                              DWORD in_buffer_size,
                                              DWORD default_timeout,
                                              HANDLE* pipe) {
+  *pipe = INVALID_HANDLE_VALUE;
   // The only action supported is ASK_BROKER which means create the pipe.
   if (ASK_BROKER != eval_result) {
     return ERROR_ACCESS_DENIED;
   }
 
   *pipe = CreateNamedPipeHelper(client_info.process, name.c_str(),
                                 open_mode, pipe_mode, max_instances,
                                 out_buffer_size, in_buffer_size,
--- a/security/sandbox/chromium/sandbox/win/src/process_thread_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/process_thread_policy.cc
@@ -248,16 +248,17 @@ DWORD ProcessPolicy::CreateProcessWActio
 DWORD ProcessPolicy::CreateThreadAction(
     const ClientInfo& client_info,
     const SIZE_T stack_size,
     const LPTHREAD_START_ROUTINE start_address,
     const LPVOID parameter,
     const DWORD creation_flags,
     LPDWORD thread_id,
     HANDLE* handle) {
+  *handle = NULL;
   HANDLE local_handle =
       ::CreateRemoteThread(client_info.process, nullptr, stack_size,
                            start_address, parameter, creation_flags, thread_id);
   if (!local_handle) {
     return ::GetLastError();
   }
   if (!::DuplicateHandle(::GetCurrentProcess(), local_handle,
                          client_info.process, handle, 0, FALSE,
--- a/security/sandbox/chromium/sandbox/win/src/registry_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/registry_policy.cc
@@ -57,16 +57,17 @@ NTSTATUS TranslateMaximumAllowed(OBJECT_
 NTSTATUS NtCreateKeyInTarget(HANDLE* target_key_handle,
                              ACCESS_MASK desired_access,
                              OBJECT_ATTRIBUTES* obj_attributes,
                              ULONG title_index,
                              UNICODE_STRING* class_name,
                              ULONG create_options,
                              ULONG* disposition,
                              HANDLE target_process) {
+  *target_key_handle = nullptr;
   NtCreateKeyFunction NtCreateKey = NULL;
   ResolveNTFunctionPtr("NtCreateKey", &NtCreateKey);
 
   if (MAXIMUM_ALLOWED & desired_access) {
     NTSTATUS status = TranslateMaximumAllowed(obj_attributes, &desired_access);
     if (!NT_SUCCESS(status))
       return STATUS_ACCESS_DENIED;
   }
@@ -85,16 +86,17 @@ NTSTATUS NtCreateKeyInTarget(HANDLE* tar
   }
   return STATUS_SUCCESS;
 }
 
 NTSTATUS NtOpenKeyInTarget(HANDLE* target_key_handle,
                            ACCESS_MASK desired_access,
                            OBJECT_ATTRIBUTES* obj_attributes,
                            HANDLE target_process) {
+  *target_key_handle = nullptr;
   NtOpenKeyFunction NtOpenKey = NULL;
   ResolveNTFunctionPtr("NtOpenKey", &NtOpenKey);
 
   if (MAXIMUM_ALLOWED & desired_access) {
     NTSTATUS status = TranslateMaximumAllowed(obj_attributes, &desired_access);
     if (!NT_SUCCESS(status))
       return STATUS_ACCESS_DENIED;
   }
@@ -208,17 +210,17 @@ bool RegistryPolicy::OpenKeyAction(EvalR
                                    HANDLE root_directory,
                                    uint32_t desired_access,
                                    HANDLE* handle,
                                    NTSTATUS* nt_status) {
   // The only action supported is ASK_BROKER which means open the requested
   // file as specified.
   if (ASK_BROKER != eval_result) {
     *nt_status = STATUS_ACCESS_DENIED;
-    return true;
+    return false;
   }
 
   UNICODE_STRING uni_name = {0};
   OBJECT_ATTRIBUTES obj_attributes = {0};
   InitObjectAttribs(key, attributes, root_directory, &obj_attributes,
                     &uni_name, NULL);
   *nt_status = NtOpenKeyInTarget(handle, desired_access, &obj_attributes,
                                 client_info.process);