Bug 1230910 Port complicated __try blocks to __try1 draft
authorTom Ritter <tom@mozilla.com>
Wed, 30 Aug 2017 12:55:59 -0500
changeset 656093 b436c72c4ec1eedc1cf7d5fcf9875c65cd41bdb7
parent 656092 404f495cc931d97f86e377087bd34fb7d96ba458
child 656094 3189cea122aa3d3ff3278845da9d90423e49fe51
push id77058
push userbmo:tom@mozilla.com
push dateWed, 30 Aug 2017 18:21:00 +0000
bugs1230910
milestone57.0a1
Bug 1230910 Port complicated __try blocks to __try1 Not sure if __try1 will be able to handle these or not... MozReview-Commit-ID: 6iXeuYrP2S6
security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc
security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
@@ -343,25 +343,39 @@ NTSTATUS WINAPI TargetNtSetInformationFi
       break;
 
     FILE_RENAME_INFORMATION* file_rename_info =
         reinterpret_cast<FILE_RENAME_INFORMATION*>(file_info);
     OBJECT_ATTRIBUTES object_attributes;
     UNICODE_STRING object_name;
     InitializeObjectAttributes(&object_attributes, &object_name, 0, NULL, NULL);
 
-    __try {
+    /*
+     * This one is particurally tricky. It's an interceptor of NtSetInformationFile
+     * on the child process, which can be called in ways we probably can't reasonably 
+     * identify. The except handler will return STATUS_ACCESS_DENIED to the caller.
+     *
+     * It seems possible that any of the multitude of places this could be called
+     * will both:
+     * a) Make a request of the sandbox to fiddle a file it doesn't have access to
+     *    according to IsSupportedRenameCall()
+     * b) intelligently handle a STATUS_ACCESS_DENIED result
+     *
+     * Therefore, we don't want to abort() here, we want to handle it intelligently
+     * too. Do so with a __try1/__except1
+     */
+    __try1(ehandler) {
       if (!IsSupportedRenameCall(file_rename_info, length, file_info_class))
         break;
 
       object_attributes.RootDirectory = file_rename_info->RootDirectory;
       object_name.Buffer = file_rename_info->FileName;
       object_name.Length = object_name.MaximumLength =
           static_cast<USHORT>(file_rename_info->FileNameLength);
-    } __except(EXCEPTION_EXECUTE_HANDLER) {
+    } __except1 {
       break;
     }
 
     NTSTATUS ret = AllocAndCopyName(&object_attributes, &name, NULL, NULL);
     if (!NT_SUCCESS(ret) || !name)
       break;
 
     uint32_t broker = FALSE;
--- a/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc
+++ b/security/sandbox/chromium/sandbox/win/src/process_thread_interception.cc
@@ -463,27 +463,27 @@ HANDLE WINAPI TargetCreateThread(CreateT
   do {
     if (NULL == target_services)
       break;
 
     // We don't trust that the IPC can work this early.
     if (!target_services->GetState()->InitCalled())
       break;
 
-    __try {
+    __try1(ehandler) {
       if (NULL != thread_id &&
           !ValidParameter(thread_id, sizeof(*thread_id), WRITE))
         break;
 
       if (nullptr == start_address)
         break;
       // We don't support thread_attributes not being null.
       if (nullptr != thread_attributes)
         break;
-    } __except (EXCEPTION_EXECUTE_HANDLER) {
+    } __except1 {
       break;
     }
 
     void* memory = GetGlobalIPCMemory();
     if (nullptr == memory)
       break;
 
     SharedMemIPCClient ipc(memory);
@@ -498,22 +498,22 @@ HANDLE WINAPI TargetCreateThread(CreateT
     if (SBOX_ALL_OK != code)
       break;
 
     ::SetLastError(answer.win32_result);
     if (ERROR_SUCCESS != answer.win32_result) {
       return NULL;
     }
 
-    __try {
+    __try1(ehandler) {
       if (thread_id != NULL) {
         *thread_id = ::GetThreadId(answer.handle);
       }
       return answer.handle;
-    } __except (EXCEPTION_EXECUTE_HANDLER) {
+    } __except1 {
       break;
     }
   } while (false);
 
   ::SetLastError(original_error);
   return NULL;
 }
 
--- a/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc
+++ b/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc
@@ -234,17 +234,17 @@ NTSTATUS AllocAndGetFullPath(HANDLE root
   if (!InitHeap())
     return STATUS_NO_MEMORY;
 
   DCHECK_NT(full_path);
   DCHECK_NT(path);
   *full_path = NULL;
   OBJECT_NAME_INFORMATION* handle_name = NULL;
   NTSTATUS ret = STATUS_UNSUCCESSFUL;
-  __try {
+  __try1(ehandler) {
     do {
       static NtQueryObjectFunction NtQueryObject = NULL;
       if (!NtQueryObject)
         ResolveNTFunctionPtr("NtQueryObject", &NtQueryObject);
 
       ULONG size = 0;
       // Query the name information a first time to get the size of the name.
       ret = NtQueryObject(root, ObjectNameInformation, NULL, 0, &size);
@@ -277,17 +277,17 @@ NTSTATUS AllocAndGetFullPath(HANDLE root
       *off = L'\\';
       off += 1;
       ret = CopyData(off, path, wcslen(path) * sizeof(wchar_t));
       if (!NT_SUCCESS(ret))
         break;
       off += wcslen(path);
       *off = L'\0';
     } while (false);
-  } __except(EXCEPTION_EXECUTE_HANDLER) {
+  } __except1 {
     ret = GetExceptionCode();
   }
 
   if (!NT_SUCCESS(ret)) {
     if (*full_path) {
       operator delete(*full_path, NT_ALLOC);
       *full_path = NULL;
     }
@@ -306,17 +306,17 @@ NTSTATUS AllocAndCopyName(const OBJECT_A
                           uint32_t* attributes,
                           HANDLE* root) {
   if (!InitHeap())
     return STATUS_NO_MEMORY;
 
   DCHECK_NT(out_name);
   *out_name = NULL;
   NTSTATUS ret = STATUS_UNSUCCESSFUL;
-  __try {
+  __try1(ehandler) {
     do {
       if (in_object->RootDirectory != static_cast<HANDLE>(0) && !root)
         break;
       if (NULL == in_object->ObjectName)
         break;
       if (NULL == in_object->ObjectName->Buffer)
         break;
 
@@ -334,17 +334,17 @@ NTSTATUS AllocAndCopyName(const OBJECT_A
 
       if (attributes)
         *attributes = in_object->Attributes;
 
       if (root)
         *root = in_object->RootDirectory;
       ret = STATUS_SUCCESS;
     } while (false);
-  } __except(EXCEPTION_EXECUTE_HANDLER) {
+  } __except1 {
     ret = GetExceptionCode();
   }
 
   if (!NT_SUCCESS(ret) && *out_name) {
     operator delete(*out_name, NT_ALLOC);
     *out_name = NULL;
   }
 
@@ -446,17 +446,17 @@ UNICODE_STRING* AnsiToUnicode(const char
   return out_string;
 }
 
 UNICODE_STRING* GetImageInfoFromModule(HMODULE module, uint32_t* flags) {
   // PEImage's dtor won't be run during SEH unwinding, but that's OK.
 #pragma warning(push)
 #pragma warning(disable: 4509)
   UNICODE_STRING* out_name = NULL;
-  __try {
+  __try1(ehandler) {
     do {
       *flags = 0;
       base::win::PEImage pe(module);
 
       if (!pe.VerifyMagic())
         break;
       *flags |= MODULE_IS_PE_IMAGE;
 
@@ -469,17 +469,17 @@ UNICODE_STRING* GetImageInfoFromModule(H
       PIMAGE_NT_HEADERS headers = pe.GetNTHeaders();
       if (headers) {
         if (headers->OptionalHeader.AddressOfEntryPoint)
           *flags |= MODULE_HAS_ENTRY_POINT;
         if (headers->OptionalHeader.SizeOfCode)
           *flags |= MODULE_HAS_CODE;
       }
     } while (false);
-  } __except(EXCEPTION_EXECUTE_HANDLER) {
+  } __except1 {
   }
 
   return out_name;
 #pragma warning(pop)
 }
 
 UNICODE_STRING* GetBackingFilePath(PVOID address) {
   // We'll start with something close to max_path charactes for the name.