Bug 1230910 Port complicated __try blocks to __try1 draft
authorTom Ritter <tom@mozilla.com>
Wed, 08 Mar 2017 21:35:17 +0000
changeset 496073 249491dbdc3d6161ebb963ed6fa302499c407cca
parent 496072 efe15068d562a4e1dd6133f17d08bea6efe0513e
child 496074 5287dbbf6068e95461b1aba72fb64162fe311373
push id48518
push userbmo:tom@mozilla.com
push dateThu, 09 Mar 2017 19:30:51 +0000
bugs1230910
milestone52.0.1
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/sandbox_nt_util.cc
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
@@ -347,25 +347,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/sandbox_nt_util.cc
+++ b/security/sandbox/chromium/sandbox/win/src/sandbox_nt_util.cc
@@ -225,17 +225,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);
@@ -268,17 +268,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;
     }
@@ -297,17 +297,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;
 
@@ -325,17 +325,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;
   }
 
@@ -437,17 +437,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;
 
@@ -460,17 +460,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.