Bug 1329328 - Permit sandboxed processes to access Flash temporary files. r=bobowen
authorDavid Parks <dparks@mozilla.com>
Mon, 27 Feb 2017 14:15:52 -0800
changeset 374243 0f64b24c40c4c0773e8354d6c161a68c23500f85
parent 374242 08710fe142c895e87acdb1eaea5b420f07faee07
child 374244 4fd9849aaa21ff78550b1f5d6546aaa7bf99307d
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1329328
milestone54.0a1
Bug 1329328 - Permit sandboxed processes to access Flash temporary files. r=bobowen Allows the creation/use of temp files when the user has already green-lit the use of a file for write purposes in that folder.
security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp
security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
--- a/security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp
+++ b/security/sandbox/chromium-shim/sandbox/win/permissionsService.cpp
@@ -10,42 +10,81 @@
 #include <algorithm>
 #include <winternl.h>
 
 namespace mozilla {
 namespace sandboxing {
 
 static const std::wstring ZONE_IDENTIFIER_STR(L":ZONE.IDENTIFIER");
 static const std::wstring ZONE_ID_DATA_STR(L":ZONE.IDENTIFIER:$DATA");
+// Generic name we use for all Flash temp files.
+static const std::wstring FLASH_TEMP_FILENAME(L"FLASHTMP0.TMP");
 
 bool
 StringEndsWith(const std::wstring& str, const std::wstring& strEnding)
 {
   if (strEnding.size() > str.size()) {
     return false;
   }
   return std::equal(strEnding.rbegin(), strEnding.rend(), str.rbegin());
 }
 
+// Returns true if aFilename describes a Flash temp file.  If aFolder is
+// non-null then it is filled with the name of the folder containing
+// the file (with trailing slash).
+bool
+IsFlashTempFile(std::wstring aFilename, std::wstring* aFolder=nullptr)
+{
+  // Assume its a flash file if the base name begins with "FlashTmp",
+  // ends with ".tmp" and has an int in-between them.
+  int slashIdx = aFilename.find_last_of(L'\\');
+  if (slashIdx != std::wstring::npos) {
+    if (aFolder) {
+      *aFolder = aFilename.substr(0, slashIdx + 1);
+    }
+    aFilename = aFilename.substr(slashIdx + 1);
+  } else {
+    *aFolder = L"\\";
+  }
+
+  if (aFilename.compare(0, 8, L"FLASHTMP") != 0) {
+    return false;
+  }
+
+  int idx = 8;
+  int len = aFilename.length();
+  while (idx < len && isdigit(aFilename[idx])) {
+    ++idx;
+  }
+
+  return (len-idx == 4) && aFilename.compare(idx, 4, L".TMP") == 0;
+}
+
 // Converts NT device internal filenames to normal user-space by stripping
-// the prefixes and suffixes from the file name.
+// the prefixes and suffixes from the file name.  Returns containing
+// folder (with trailing slash) in aFolder if non-null.
 std::wstring
-GetPlainFileName(const wchar_t* aNTFileName)
+GetPlainFileName(const wchar_t* aNTFileName, std::wstring* aFolder=nullptr)
 {
   while (*aNTFileName == L'\\' || *aNTFileName == L'.' ||
          *aNTFileName == L'?' || *aNTFileName == L':' ) {
     ++aNTFileName;
   }
   std::wstring nameCopy(aNTFileName);
   std::transform(nameCopy.begin(), nameCopy.end(), nameCopy.begin(), towupper);
   if (StringEndsWith(nameCopy, ZONE_ID_DATA_STR)) {
     nameCopy = nameCopy.substr(0, nameCopy.size() - ZONE_ID_DATA_STR.size());
   } else if (StringEndsWith(nameCopy, ZONE_IDENTIFIER_STR)) {
     nameCopy = nameCopy.substr(0, nameCopy.size() - ZONE_IDENTIFIER_STR.size());
   }
+
+  if (IsFlashTempFile(nameCopy, aFolder) && aFolder) {
+    return *aFolder + FLASH_TEMP_FILENAME;
+  }
+
   return nameCopy;
 }
 
 /* static */ PermissionsService*
 PermissionsService::GetInstance()
 {
   static PermissionsService sPermissionsService;
   return &sPermissionsService;
@@ -57,18 +96,23 @@ PermissionsService::PermissionsService()
 }
 
 void
 PermissionsService::GrantFileAccess(uint32_t aProcessId,
                                     const wchar_t* aFilename,
                                     bool aPermitWrite)
 {
   FilePermissionMap& permissions = mProcessFilePermissions[aProcessId];
-  std::wstring filename = GetPlainFileName(aFilename);
+  std::wstring containingFolder;
+  std::wstring filename = GetPlainFileName(aFilename, &containingFolder);
   permissions[filename] |= aPermitWrite;
+  if (aPermitWrite) {
+    // Also grant write permission to FLASH_TEMP_FILENAME in the same folder.
+    permissions[containingFolder + FLASH_TEMP_FILENAME] = true;
+  }
 }
 
 void
 PermissionsService::SetFileAccessViolationFunc(FileAccessViolationFunc aFavFunc)
 {
   mFileAccessViolationFunc = aFavFunc;
 }
 
@@ -105,16 +149,17 @@ PermissionsService::UserGrantedFileAcces
 
   auto permissions = mProcessFilePermissions.find(aProcessId);
   if (permissions == mProcessFilePermissions.end()) {
     ReportBlockedFile(needsWrite);
     return false;  // process has no special file access at all
   }
 
   std::wstring filename = GetPlainFileName(aFilename);
+
   auto itPermission = permissions->second.find(filename);
   if (itPermission == permissions->second.end()) {
     ReportBlockedFile(needsWrite);
     return false;  // process has no access to this file
   }
 
   // We have read permission.  Check for write permission if requested.
   if (!needsWrite || itPermission->second) {
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_dispatcher.cc
@@ -221,16 +221,25 @@ bool FilesystemDispatcher::NtQueryAttrib
   params[FileName::BROKER] = ParamPickerMake(broker);
 
   // To evaluate the policy we need to call back to the policy object. We
   // are just middlemen in the operation since is the FileSystemPolicy which
   // knows what to do.
   EvalResult result = policy_base_->EvalPolicy(IPC_NTQUERYATTRIBUTESFILE_TAG,
                                                params.GetBase());
 
+  // If the policies forbid access (any result other than ASK_BROKER),
+  // then check for user-granted access to file.
+  if (ASK_BROKER != result &&
+      mozilla::sandboxing::PermissionsService::GetInstance()->
+        UserGrantedFileAccess(ipc->client_info->process_id, filename,
+                              0, 0)) {
+    result = ASK_BROKER;
+  }
+
   FILE_BASIC_INFORMATION* information =
         reinterpret_cast<FILE_BASIC_INFORMATION*>(info->Buffer());
   NTSTATUS nt_status;
   if (!FileSystemPolicy::QueryAttributesFileAction(result, *ipc->client_info,
                                                    *name, attributes,
                                                    information, &nt_status)) {
     ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
     return true;
@@ -261,16 +270,25 @@ bool FilesystemDispatcher::NtQueryFullAt
   params[FileName::BROKER] = ParamPickerMake(broker);
 
   // To evaluate the policy we need to call back to the policy object. We
   // are just middlemen in the operation since is the FileSystemPolicy which
   // knows what to do.
   EvalResult result = policy_base_->EvalPolicy(
                           IPC_NTQUERYFULLATTRIBUTESFILE_TAG, params.GetBase());
 
+  // If the policies forbid access (any result other than ASK_BROKER),
+  // then check for user-granted access to file.
+  if (ASK_BROKER != result &&
+      mozilla::sandboxing::PermissionsService::GetInstance()->
+        UserGrantedFileAccess(ipc->client_info->process_id, filename,
+                              0, 0)) {
+    result = ASK_BROKER;
+  }
+
   FILE_NETWORK_OPEN_INFORMATION* information =
         reinterpret_cast<FILE_NETWORK_OPEN_INFORMATION*>(info->Buffer());
   NTSTATUS nt_status;
   if (!FileSystemPolicy::QueryFullAttributesFileAction(result,
                                                        *ipc->client_info,
                                                        *name, attributes,
                                                        information,
                                                        &nt_status)) {
@@ -316,16 +334,26 @@ bool FilesystemDispatcher::NtSetInformat
   params[FileName::BROKER] = ParamPickerMake(broker);
 
   // To evaluate the policy we need to call back to the policy object. We
   // are just middlemen in the operation since is the FileSystemPolicy which
   // knows what to do.
   EvalResult result = policy_base_->EvalPolicy(IPC_NTSETINFO_RENAME_TAG,
                                                params.GetBase());
 
+  // If the policies forbid access (any result other than ASK_BROKER),
+  // then check for user-granted write access to file.  We only permit
+  // the FileRenameInformation action.
+  if (ASK_BROKER != result && info_class == FileRenameInformation &&
+      mozilla::sandboxing::PermissionsService::GetInstance()->
+        UserGrantedFileAccess(ipc->client_info->process_id, filename,
+                              FILE_WRITE_ATTRIBUTES, 0)) {
+    result = ASK_BROKER;
+  }
+
   IO_STATUS_BLOCK* io_status =
         reinterpret_cast<IO_STATUS_BLOCK*>(status->Buffer());
   NTSTATUS nt_status;
   if (!FileSystemPolicy::SetInformationFileAction(result, *ipc->client_info,
                                                   handle, rename_info, length,
                                                   info_class, io_status,
                                                   &nt_status)) {
     ipc->return_info.nt_status = STATUS_ACCESS_DENIED;
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
@@ -216,19 +216,16 @@ NTSTATUS WINAPI TargetNtQueryAttributesF
     InOutCountedBuffer file_info(file_attributes,
                                  sizeof(FILE_BASIC_INFORMATION));
 
     uint32_t broker = FALSE;
     CountedParameterSet<FileName> params;
     params[FileName::NAME] = ParamPickerMake(name);
     params[FileName::BROKER] = ParamPickerMake(broker);
 
-    if (!QueryBroker(IPC_NTQUERYATTRIBUTESFILE_TAG, params.GetBase()))
-      break;
-
     SharedMemIPCClient ipc(memory);
     CrossCallReturn answer = {0};
     ResultCode code = CrossCall(ipc, IPC_NTQUERYATTRIBUTESFILE_TAG, name,
                                 attributes, file_info, &answer);
 
     if (SBOX_ALL_OK != code)
       break;
 
@@ -282,19 +279,16 @@ NTSTATUS WINAPI TargetNtQueryFullAttribu
     InOutCountedBuffer file_info(file_attributes,
                                  sizeof(FILE_NETWORK_OPEN_INFORMATION));
 
     uint32_t broker = FALSE;
     CountedParameterSet<FileName> params;
     params[FileName::NAME] = ParamPickerMake(name);
     params[FileName::BROKER] = ParamPickerMake(broker);
 
-    if (!QueryBroker(IPC_NTQUERYFULLATTRIBUTESFILE_TAG, params.GetBase()))
-      break;
-
     SharedMemIPCClient ipc(memory);
     CrossCallReturn answer = {0};
     ResultCode code = CrossCall(ipc, IPC_NTQUERYFULLATTRIBUTESFILE_TAG, name,
                                 attributes, file_info, &answer);
 
     if (SBOX_ALL_OK != code)
       break;
 
@@ -361,19 +355,16 @@ NTSTATUS WINAPI TargetNtSetInformationFi
     if (!NT_SUCCESS(ret) || !name)
       break;
 
     uint32_t broker = FALSE;
     CountedParameterSet<FileName> params;
     params[FileName::NAME] = ParamPickerMake(name);
     params[FileName::BROKER] = ParamPickerMake(broker);
 
-    if (!QueryBroker(IPC_NTSETINFO_RENAME_TAG, params.GetBase()))
-      break;
-
     InOutCountedBuffer io_status_buffer(io_status, sizeof(IO_STATUS_BLOCK));
     // This is actually not an InOut buffer, only In, but using InOut facility
     // really helps to simplify the code.
     InOutCountedBuffer file_info_buffer(file_info, length);
 
     SharedMemIPCClient ipc(memory);
     CrossCallReturn answer = {0};
     ResultCode code = CrossCall(ipc, IPC_NTSETINFO_RENAME_TAG, file,