Bug 1510494 - write elevated updater log and status files to a new directory in the Maintenance Service directory. r=agashlin,mhowell

Differential Revision: https://phabricator.services.mozilla.com/D46627

--HG--
extra : moz-landing-system : lando

toolkit/components/maintenanceservice/maintenanceservice.cpp

@@ -42,9 +42,10 @@ int wmain(int argc, WCHAR** argv) {
   // Otherwise, the service is probably being started by the SCM.
   bool forceInstall = !lstrcmpi(argv[1], L"forceinstall");
   if (!lstrcmpi(argv[1], L"install") || forceInstall) {
-    WCHAR updatePath[MAX_PATH + 1];
-    if (GetLogDirectoryPath(updatePath)) {
-      LogInit(updatePath, L"maintenanceservice-install.log");
+    WCHAR logFilePath[MAX_PATH + 1];
+    if (GetLogDirectoryPath(logFilePath) &&
+        PathAppendSafe(logFilePath, L"maintenanceservice-install.log")) {
+      LogInit(logFilePath);
     }
 
     SvcInstallAction action = InstallSvc;
@@ -68,9 +69,10 @@ int wmain(int argc, WCHAR** argv) {
   }
 
   if (!lstrcmpi(argv[1], L"upgrade")) {
-    WCHAR updatePath[MAX_PATH + 1];
-    if (GetLogDirectoryPath(updatePath)) {
-      LogInit(updatePath, L"maintenanceservice-install.log");
+    WCHAR logFilePath[MAX_PATH + 1];
+    if (GetLogDirectoryPath(logFilePath) &&
+        PathAppendSafe(logFilePath, L"maintenanceservice-install.log")) {
+      LogInit(logFilePath);
     }
 
     LOG(("Upgrading service if installed..."));
@@ -86,9 +88,10 @@ int wmain(int argc, WCHAR** argv) {
   }
 
   if (!lstrcmpi(argv[1], L"uninstall")) {
-    WCHAR updatePath[MAX_PATH + 1];
-    if (GetLogDirectoryPath(updatePath)) {
-      LogInit(updatePath, L"maintenanceservice-uninstall.log");
+    WCHAR logFilePath[MAX_PATH + 1];
+    if (GetLogDirectoryPath(logFilePath) &&
+        PathAppendSafe(logFilePath, L"maintenanceservice-uninstall.log")) {
+      LogInit(logFilePath);
     }
     LOG(("Uninstalling service..."));
     if (!SvcUninstall()) {
@@ -226,10 +229,12 @@ void StartTerminationThread() {
  */
 void WINAPI SvcMain(DWORD argc, LPWSTR* argv) {
   // Setup logging, and backup the old logs
-  WCHAR updatePath[MAX_PATH + 1];
-  if (GetLogDirectoryPath(updatePath)) {
-    BackupOldLogs(updatePath, LOGS_TO_KEEP);
-    LogInit(updatePath, L"maintenanceservice.log");
+  WCHAR logFilePath[MAX_PATH + 1];
+  if (GetLogDirectoryPath(logFilePath)) {
+    BackupOldLogs(logFilePath, LOGS_TO_KEEP);
+    if (PathAppendSafe(logFilePath, L"maintenanceservice.log")) {
+      LogInit(logFilePath);
+    }
   }
 
   // Disable every privilege we don't need. Processes started using

toolkit/components/maintenanceservice/workmonitor.cpp

@@ -32,8 +32,6 @@ using mozilla::UniquePtr;
 #include "updatererrors.h"
 #include "commonupdatedir.h"
 
-#define PATCH_DIR_PATH L"\\updates\\0"
-
 // Wait 15 minutes for an update operation to run at most.
 // Updates usually take less than a minute so this seems like a
 // significantly large and safe amount of time to wait.
@@ -43,25 +41,25 @@ BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer, LPCWSTR siblingFilePath,
 BOOL DoesFallbackKeyExist();
 
 /*
- * Read the update.status file and sets isApplying to true if
- * the status is set to applying.
+ * Reads the secure update status file and sets isApplying to true if the status
+ * is set to applying.
  *
- * @param  updateDirPath The directory where update.status is stored
+ * @param  patchDirPath
+ *         The update patch directory path
  * @param  isApplying Out parameter for specifying if the status
  *         is set to applying or not.
  * @return TRUE if the information was filled.
  */
-static BOOL IsStatusApplying(LPCWSTR updateDirPath, BOOL& isApplying) {
+static BOOL IsStatusApplying(LPCWSTR patchDirPath, BOOL& isApplying) {
   isApplying = FALSE;
-  WCHAR updateStatusFilePath[MAX_PATH + 1] = {L'\0'};
-  wcsncpy(updateStatusFilePath, updateDirPath, MAX_PATH);
-  if (!PathAppendSafe(updateStatusFilePath, L"update.status")) {
-    LOG_WARN(("Could not append path for update.status file"));
+  WCHAR statusFilePath[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFilePath(patchDirPath, L".status", statusFilePath)) {
+    LOG_WARN(("Could not get path for the secure update status file"));
     return FALSE;
   }
 
   nsAutoHandle statusFile(
-      CreateFileW(updateStatusFilePath, GENERIC_READ,
+      CreateFileW(statusFilePath, GENERIC_READ,
                   FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
                   nullptr, OPEN_EXISTING, 0, nullptr));
 
@@ -585,18 +583,12 @@ BOOL ExecuteServiceCommand(int argc, LPWSTR* argv) {
 
   // The tests work by making sure the log has changed, so we put a
   // unique ID in the log.
-  GUID guid;
-  HRESULT hr = CoCreateGuid(&guid);
-  if (SUCCEEDED(hr)) {
-    RPC_WSTR guidString = RPC_WSTR(L"");
-    if (UuidToString(&guid, &guidString) == RPC_S_OK) {
-      LOG(("Executing service command %ls, ID: %ls", argv[2],
-           reinterpret_cast<LPCWSTR>(guidString)));
-      RpcStringFree(&guidString);
-    } else {
-      // The ID is only used by tests, so failure to allocate it isn't fatal.
-      LOG(("Executing service command %ls", argv[2]));
-    }
+  WCHAR uuidString[MAX_PATH + 1] = {L'\0'};
+  if (GetUUIDString(uuidString)) {
+    LOG(("Executing service command %ls, ID: %ls", argv[2], uuidString));
+  } else {
+    // The ID is only used by tests, so failure to allocate it isn't fatal.
+    LOG(("Executing service command %ls", argv[2]));
   }
 
   BOOL result = FALSE;
@@ -629,6 +621,16 @@ BOOL ExecuteServiceCommand(int argc, LPWSTR* argv) {
       return FALSE;
     }
 
+    // Remove the secure output files so it is easier to determine when new
+    // files are created in the unelevated updater.
+    RemoveSecureOutputFiles(argv[4]);
+
+    // Create a new secure ID for this update.
+    if (!WriteSecureIDFile(argv[4])) {
+      LOG_WARN(("Unable to write to secure ID file."));
+      return FALSE;
+    }
+
     // This check is also performed in updater.cpp and is performed here
     // as well since the maintenance service can be called directly.
     if (argc < 5 || !IsValidFullPath(argv[5])) {

toolkit/mozapps/update/common/updatecommon.cpp

@@ -49,27 +49,17 @@ typedef struct _REPARSE_DATA_BUFFER {
 
 UpdateLog::UpdateLog() : logFP(nullptr) {}
 
-void UpdateLog::Init(NS_tchar* sourcePath, const NS_tchar* fileName) {
+void UpdateLog::Init(NS_tchar* logFilePath) {
   if (logFP) {
     return;
   }
 
-  int dstFilePathLen =
-      NS_tsnprintf(mDstFilePath, sizeof(mDstFilePath) / sizeof(mDstFilePath[0]),
-                   NS_T("%s/%s"), sourcePath, fileName);
-  // If the destination path was over the length limit,
-  // disable logging by skipping opening the file and setting logFP.
-  if ((dstFilePathLen > 0) &&
-      (dstFilePathLen <
-       static_cast<int>(sizeof(mDstFilePath) / sizeof(mDstFilePath[0])))) {
-#ifdef XP_WIN
-    if (GetUUIDTempFilePath(sourcePath, L"log", mTmpFilePath)) {
-      logFP = NS_tfopen(mTmpFilePath, NS_T("w"));
-      // Delete this file now so it is possible to tell from the unelevated
-      // updater process if the elevated updater process has written the log.
-      DeleteFileW(mDstFilePath);
-    }
-#elif XP_MACOSX
+  // When the path is over the length limit disable logging by not opening the
+  // file and not setting logFP.
+  int dstFilePathLen = NS_tstrlen(logFilePath);
+  if (dstFilePathLen > 0 && dstFilePathLen < MAXPATHLEN - 1) {
+    NS_tstrncpy(mDstFilePath, logFilePath, MAXPATHLEN);
+#if defined(XP_WIN) || defined(XP_MACOSX)
     logFP = NS_tfopen(mDstFilePath, NS_T("w"));
 #else
     // On platforms that have an updates directory in the installation directory
@@ -126,16 +116,6 @@ void UpdateLog::Finish() {
 
   fclose(logFP);
   logFP = nullptr;
-
-#ifdef XP_WIN
-  // When the log file already exists then the elevated updater has already
-  // written the log file and the temp file for the log should be discarded.
-  if (!NS_taccess(mDstFilePath, F_OK)) {
-    DeleteFileW(mTmpFilePath);
-  } else {
-    MoveFileW(mTmpFilePath, mDstFilePath);
-  }
-#endif
 }
 
 void UpdateLog::Flush() {

toolkit/mozapps/update/common/updatecommon.h

@@ -16,7 +16,7 @@ class UpdateLog {
     return primaryLog;
   }
 
-  void Init(NS_tchar* sourcePath, const NS_tchar* fileName);
+  void Init(NS_tchar* logFilePath);
   void Finish();
   void Flush();
   void Printf(const char* fmt, ...) MOZ_FORMAT_PRINTF(2, 3);
@@ -27,7 +27,6 @@ class UpdateLog {
  protected:
   UpdateLog();
   FILE* logFP;
-  NS_tchar mTmpFilePath[MAXPATHLEN];
   NS_tchar mDstFilePath[MAXPATHLEN];
 };
 
@@ -35,8 +34,7 @@ bool IsValidFullPath(NS_tchar* fullPath);
 
 #define LOG_WARN(args) UpdateLog::GetPrimaryLog().WarnPrintf args
 #define LOG(args) UpdateLog::GetPrimaryLog().Printf args
-#define LogInit(PATHNAME_, FILENAME_) \
-  UpdateLog::GetPrimaryLog().Init(PATHNAME_, FILENAME_)
+#define LogInit(FILEPATH_) UpdateLog::GetPrimaryLog().Init(FILEPATH_)
 #define LogFinish() UpdateLog::GetPrimaryLog().Finish()
 #define LogFlush() UpdateLog::GetPrimaryLog().Flush()
 

toolkit/mozapps/update/common/updatehelper.cpp

@@ -9,6 +9,7 @@
 #ifndef ONLY_SERVICE_LAUNCHING
 
 #  include <stdio.h>
+#  include <direct.h>
 #  include "mozilla/UniquePtr.h"
 #  include "pathhash.h"
 #  include "shlobj.h"
@@ -36,20 +37,218 @@ BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer, LPCWSTR siblingFilePath,
  */
 BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer, LPCWSTR siblingFilePath,
                             LPCWSTR newFileName) {
-  if (wcslen(siblingFilePath) >= MAX_PATH) {
+  if (wcslen(siblingFilePath) > MAX_PATH) {
     return FALSE;
   }
 
-  wcsncpy(destinationBuffer, siblingFilePath, MAX_PATH);
+  wcsncpy(destinationBuffer, siblingFilePath, MAX_PATH + 1);
   if (!PathRemoveFileSpecW(destinationBuffer)) {
     return FALSE;
   }
 
-  if (wcslen(destinationBuffer) + wcslen(newFileName) >= MAX_PATH) {
+  return PathAppendSafe(destinationBuffer, newFileName);
+}
+
+/**
+ * Obtains the path of the secure directory used to write the status and log
+ * files for updates applied with an elevated updater or an updater that is
+ * launched using the maintenance service.
+ *
+ * Example
+ * Destination buffer value:
+ *   C:\Program Files (x86)\Mozilla Maintenance Service\UpdateLogs
+ *
+ * @param  outBuf
+ *         A buffer of size MAX_PATH + 1 to store the result.
+ * @return TRUE if successful
+ */
+BOOL GetSecureOutputDirectoryPath(LPWSTR outBuf) {
+  PWSTR progFilesX86;
+  if (FAILED(SHGetKnownFolderPath(FOLDERID_ProgramFilesX86, KF_FLAG_CREATE,
+                                  nullptr, &progFilesX86))) {
+    return FALSE;
+  }
+  if (wcslen(progFilesX86) > MAX_PATH) {
+    CoTaskMemFree(progFilesX86);
+    return FALSE;
+  }
+  wcsncpy(outBuf, progFilesX86, MAX_PATH + 1);
+  CoTaskMemFree(progFilesX86);
+
+  if (!PathAppendSafe(outBuf, L"Mozilla Maintenance Service")) {
     return FALSE;
   }
 
-  return PathAppendSafe(destinationBuffer, newFileName);
+  // Create the Maintenance Service directory in case it doesn't exist.
+  if (!CreateDirectoryW(outBuf, nullptr) &&
+      GetLastError() != ERROR_ALREADY_EXISTS) {
+    return FALSE;
+  }
+
+  if (!PathAppendSafe(outBuf, L"UpdateLogs")) {
+    return FALSE;
+  }
+
+  // Create the secure update output directory in case it doesn't exist.
+  if (!CreateDirectoryW(outBuf, nullptr) &&
+      GetLastError() != ERROR_ALREADY_EXISTS) {
+    return FALSE;
+  }
+
+  return TRUE;
+}
+
+/**
+ * Obtains the name of the update output file using the update patch directory
+ * path and file extension (must include the '.' separator) passed to this
+ * function.
+ *
+ * Example
+ * Patch directory path parameter:
+ *   C:\ProgramData\Mozilla\updates\0123456789ABCDEF\updates\0
+ * File extension parameter:
+ *   .status
+ * Destination buffer value:
+ *   0123456789ABCDEF.status
+ *
+ * @param  patchDirPath
+ *         The path to the update patch directory.
+ * @param  fileExt
+ *         The file extension for the file including the '.' separator.
+ * @param  outBuf
+ *         A buffer of size MAX_PATH + 1 to store the result.
+ * @return TRUE if successful
+ */
+BOOL GetSecureOutputFileName(LPCWSTR patchDirPath, LPCWSTR fileExt,
+                             LPWSTR outBuf) {
+  size_t fullPathLen = wcslen(patchDirPath);
+  if (fullPathLen > MAX_PATH) {
+    return FALSE;
+  }
+
+  size_t relPathLen = wcslen(PATCH_DIR_PATH);
+  if (relPathLen > fullPathLen) {
+    return FALSE;
+  }
+
+  // The patch directory path must end with updates\0 for updates applied with
+  // an elevated updater or an updater that is launched using the maintenance
+  // service.
+  if (_wcsnicmp(patchDirPath + fullPathLen - relPathLen, PATCH_DIR_PATH,
+                relPathLen) != 0) {
+    return FALSE;
+  }
+
+  wcsncpy(outBuf, patchDirPath, MAX_PATH + 1);
+  if (!PathRemoveFileSpecW(outBuf)) {
+    return FALSE;
+  }
+
+  if (!PathRemoveFileSpecW(outBuf)) {
+    return FALSE;
+  }
+
+  PathStripPathW(outBuf);
+
+  size_t outBufLen = wcslen(outBuf);
+  size_t fileExtLen = wcslen(fileExt);
+  if (outBufLen + fileExtLen > MAX_PATH) {
+    return FALSE;
+  }
+
+  wcsncat(outBuf, fileExt, fileExtLen);
+
+  return TRUE;
+}
+
+/**
+ * Obtains the full path of the secure update output file using the update patch
+ * directory path and file extension (must include the '.' separator) passed to
+ * this function.
+ *
+ * Example
+ * Patch directory path parameter:
+ *   C:\ProgramData\Mozilla\updates\0123456789ABCDEF\updates\0
+ * File extension parameter:
+ *   .status
+ * Destination buffer value:
+ *   C:\Program Files (x86)\Mozilla Maintenance
+ *     Service\UpdateLogs\0123456789ABCDEF.status
+ *
+ * @param  patchDirPath
+ *         The path to the update patch directory.
+ * @param  fileExt
+ *         The file extension for the file including the '.' separator.
+ * @param  outBuf
+ *         A buffer of size MAX_PATH + 1 to store the result.
+ * @return TRUE if successful
+ */
+BOOL GetSecureOutputFilePath(LPCWSTR patchDirPath, LPCWSTR fileExt,
+                             LPWSTR outBuf) {
+  if (!GetSecureOutputDirectoryPath(outBuf)) {
+    return FALSE;
+  }
+
+  WCHAR statusFileName[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFileName(patchDirPath, fileExt, statusFileName)) {
+    return FALSE;
+  }
+
+  return PathAppendSafe(outBuf, statusFileName);
+}
+
+/**
+ * Writes a UUID to the ID file in the secure output directory. This is used by
+ * the unelevated updater to determine whether an existing update status file in
+ * the secure output directory has been updated.
+ *
+ * @param  patchDirPath
+ *         The path to the update patch directory.
+ * @return TRUE if successful
+ */
+BOOL WriteSecureIDFile(LPCWSTR patchDirPath) {
+  WCHAR uuidString[MAX_PATH + 1] = {L'\0'};
+  if (!GetUUIDString(uuidString)) {
+    return FALSE;
+  }
+
+  WCHAR idFilePath[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFilePath(patchDirPath, L".id", idFilePath)) {
+    return FALSE;
+  }
+
+  FILE* idFile = _wfopen(idFilePath, L"wb+");
+  if (idFile == nullptr) {
+    return FALSE;
+  }
+
+  if (fprintf(idFile, "%ls\n", uuidString) == -1) {
+    fclose(idFile);
+    return FALSE;
+  }
+
+  fclose(idFile);
+
+  return TRUE;
+}
+
+/**
+ * Removes the update status and log files from the secure output directory.
+ *
+ * @param  patchDirPath
+ *         The path to the update patch directory.
+ */
+void RemoveSecureOutputFiles(LPCWSTR patchDirPath) {
+  WCHAR filePath[MAX_PATH + 1] = {L'\0'};
+  if (GetSecureOutputFilePath(patchDirPath, L".id", filePath)) {
+    (void)_wremove(filePath);
+  }
+  if (GetSecureOutputFilePath(patchDirPath, L".status", filePath)) {
+    (void)_wremove(filePath);
+  }
+  if (GetSecureOutputFilePath(patchDirPath, L".log", filePath)) {
+    (void)_wremove(filePath);
+  }
 }
 
 /**
@@ -112,6 +311,10 @@ BOOL StartServiceUpdate(LPCWSTR installDir) {
     return FALSE;
   }
 
+  if (wcslen(installDir) > MAX_PATH) {
+    return FALSE;
+  }
+
   // Get the new maintenance service path from the install dir
   WCHAR newMaintServicePath[MAX_PATH + 1] = {L'\0'};
   wcsncpy(newMaintServicePath, installDir, MAX_PATH);
@@ -241,24 +444,25 @@ LaunchServiceSoftwareUpdateCommand(int argc, LPCWSTR* argv) {
 }
 
 /**
- * Sets update.status to a specific failure code
- *
- * @param  updateDirPath   The path of the update directory
- * @param  errorCode       Error code to set
+ * Writes a specific failure code for the update status to a file in the secure
+ * output directory. The status file's name without the '.' separator and
+ * extension is the same as the update directory name.
  *
+ * @param  patchDirPath
+ *         The path of the update patch directory.
+ * @param  errorCode
+ *         Error code to set
  * @return TRUE if successful
  */
-BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode) {
-  // The temp file is not removed on failure since there is client code that
-  // will remove it.
-  WCHAR tmpUpdateStatusFilePath[MAX_PATH + 1] = {L'\0'};
-  if (!GetUUIDTempFilePath(updateDirPath, L"svc", tmpUpdateStatusFilePath)) {
+BOOL WriteStatusFailure(LPCWSTR patchDirPath, int errorCode) {
+  WCHAR statusFilePath[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFilePath(patchDirPath, L".status", statusFilePath)) {
     return FALSE;
   }
 
-  HANDLE tmpStatusFile = CreateFileW(tmpUpdateStatusFilePath, GENERIC_WRITE, 0,
-                                     nullptr, CREATE_ALWAYS, 0, nullptr);
-  if (tmpStatusFile == INVALID_HANDLE_VALUE) {
+  HANDLE hStatusFile = CreateFileW(statusFilePath, GENERIC_WRITE, 0, nullptr,
+                                   CREATE_ALWAYS, 0, nullptr);
+  if (hStatusFile == INVALID_HANDLE_VALUE) {
     return FALSE;
   }
 
@@ -266,24 +470,13 @@ BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode) {
   sprintf(failure, "failed: %d", errorCode);
   DWORD toWrite = strlen(failure);
   DWORD wrote;
-  BOOL ok = WriteFile(tmpStatusFile, failure, toWrite, &wrote, nullptr);
-  CloseHandle(tmpStatusFile);
+  BOOL ok = WriteFile(hStatusFile, failure, toWrite, &wrote, nullptr);
+  CloseHandle(hStatusFile);
 
   if (!ok || wrote != toWrite) {
     return FALSE;
   }
 
-  WCHAR updateStatusFilePath[MAX_PATH + 1] = {L'\0'};
-  wcsncpy(updateStatusFilePath, updateDirPath, MAX_PATH);
-  if (!PathAppendSafe(updateStatusFilePath, L"update.status")) {
-    return FALSE;
-  }
-
-  if (MoveFileExW(tmpUpdateStatusFilePath, updateStatusFilePath,
-                  MOVEFILE_REPLACE_EXISTING) == 0) {
-    return FALSE;
-  }
-
   return TRUE;
 }
 

toolkit/mozapps/update/common/updatehelper.h

@@ -12,6 +12,13 @@ BOOL DoesFallbackKeyExist();
 BOOL IsLocalFile(LPCWSTR file, BOOL& isLocal);
 DWORD StartServiceCommand(int argc, LPCWSTR* argv);
 BOOL IsUnpromptedElevation(BOOL& isUnpromptedElevation);
+BOOL GetSecureOutputDirectoryPath(LPWSTR outBuf);
+BOOL GetSecureOutputFilePath(LPCWSTR patchDirPath, LPCWSTR fileExt,
+                             LPWSTR outBuf);
+BOOL WriteSecureIDFile(LPCWSTR patchDirPath);
+void RemoveSecureOutputFiles(LPCWSTR patchDirPath);
+
+#define PATCH_DIR_PATH L"\\updates\\0"
 
 #define SVC_NAME L"MozillaMaintenance"
 

toolkit/mozapps/update/common/updatererrors.h

@@ -98,6 +98,7 @@
 #define INVALID_WORKING_DIR_PATH_ERROR 76
 #define INVALID_CALLBACK_PATH_ERROR 77
 #define INVALID_CALLBACK_DIR_ERROR 78
+#define UPDATE_STATUS_UNCHANGED 79
 
 // Error codes 80 through 99 are reserved for nsUpdateService.js
 

toolkit/mozapps/update/common/updateutils_win.cpp

@@ -88,6 +88,39 @@ BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra) {
   return PathAppendW(base, extra);
 }
 
+/**
+ * Obtains a uuid as a wide string.
+ *
+ * @param  outBuf
+ *         A buffer of size MAX_PATH + 1 to store the result.
+ * @return TRUE if successful
+ */
+BOOL GetUUIDString(LPWSTR outBuf) {
+  UUID uuid;
+  RPC_WSTR uuidString = nullptr;
+
+  // Note: the return value of UuidCreate should always be RPC_S_OK on systems
+  // after Win2K / Win2003 due to the network hardware address no longer being
+  // used to create the UUID.
+  if (UuidCreate(&uuid) != RPC_S_OK) {
+    return FALSE;
+  }
+  if (UuidToStringW(&uuid, &uuidString) != RPC_S_OK) {
+    return FALSE;
+  }
+  if (!uuidString) {
+    return FALSE;
+  }
+
+  if (wcslen(reinterpret_cast<LPCWSTR>(uuidString)) > MAX_PATH) {
+    return FALSE;
+  }
+  wcsncpy(outBuf, reinterpret_cast<LPCWSTR>(uuidString), MAX_PATH + 1);
+  RpcStringFreeW(&uuidString);
+
+  return TRUE;
+}
+
 /**
  * Build a temporary file path whose name component is a UUID.
  *
@@ -100,25 +133,27 @@ BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra) {
 BOOL GetUUIDTempFilePath(LPCWSTR basePath, LPCWSTR prefix, LPWSTR tmpPath) {
   WCHAR filename[MAX_PATH + 1] = {L"\0"};
   if (prefix) {
-    wcsncpy(filename, prefix, MAX_PATH);
+    if (wcslen(prefix) > MAX_PATH) {
+      return FALSE;
+    }
+    wcsncpy(filename, prefix, MAX_PATH + 1);
   }
 
-  UUID tmpFileNameUuid;
-  RPC_WSTR tmpFileNameString = nullptr;
-  if (UuidCreate(&tmpFileNameUuid) != RPC_S_OK) {
-    return FALSE;
-  }
-  if (UuidToStringW(&tmpFileNameUuid, &tmpFileNameString) != RPC_S_OK) {
-    return FALSE;
-  }
-  if (!tmpFileNameString) {
+  WCHAR tmpFileNameString[MAX_PATH + 1] = {L"\0"};
+  if (!GetUUIDString(tmpFileNameString)) {
     return FALSE;
   }
 
-  wcsncat(filename, (LPCWSTR)tmpFileNameString, MAX_PATH);
-  RpcStringFreeW(&tmpFileNameString);
+  size_t tmpFileNameStringLen = wcslen(tmpFileNameString);
+  if (wcslen(filename) + tmpFileNameStringLen > MAX_PATH) {
+    return FALSE;
+  }
+  wcsncat(filename, tmpFileNameString, tmpFileNameStringLen);
 
-  wcsncpy(tmpPath, basePath, MAX_PATH);
+  if (wcslen(basePath) > MAX_PATH) {
+    return FALSE;
+  }
+  wcsncpy(tmpPath, basePath, MAX_PATH + 1);
   if (!PathAppendSafe(tmpPath, filename)) {
     return FALSE;
   }

toolkit/mozapps/update/common/updateutils_win.h

@@ -36,7 +36,12 @@ DIR* opendir(const WCHAR* path);
 int closedir(DIR* dir);
 dirent* readdir(DIR* dir);
 
+// This is the length of the UUID string including null termination returned by
+// GetUUIDString.
+#define UUID_LEN 37
+
 BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
+BOOL GetUUIDString(LPWSTR outBuf);
 BOOL GetUUIDTempFilePath(LPCWSTR basePath, LPCWSTR prefix, LPWSTR tmpPath);
 
 #endif  // WINDIRENT_H__

toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js

@@ -920,6 +920,18 @@ function setupTestCommon(aAppUpdateAutoEnabled = false, aAllowBits = false) {
     );
   }
 
+  if (gIsServiceTest) {
+    let exts = ["id", "log", "status"];
+    for (let i = 0; i < exts.length; ++i) {
+      let file = getSecureOutputFile(exts[i]);
+      if (file.exists()) {
+        try {
+          file.remove(false);
+        } catch (e) {}
+      }
+    }
+  }
+
   adjustGeneralPaths();
   createWorldWritableAppUpdateDir();
 
@@ -984,6 +996,18 @@ function cleanupTestCommon() {
     getLaunchScript();
   }
 
+  if (gIsServiceTest) {
+    let exts = ["id", "log", "status"];
+    for (let i = 0; i < exts.length; ++i) {
+      let file = getSecureOutputFile(exts[i]);
+      if (file.exists()) {
+        try {
+          file.remove(false);
+        } catch (e) {}
+      }
+    }
+  }
+
   if (AppConstants.platform == "win" && MOZ_APP_BASENAME) {
     let appDir = getApplyDirFile();
     let vendor = MOZ_APP_VENDOR ? MOZ_APP_VENDOR : "Mozilla";
@@ -1504,6 +1528,36 @@ function getMaintSvcDir() {
   return maintSvcDir;
 }
 
+/**
+ * Reads the current update operation/state in the status file in the secure
+ * update log directory.
+ *
+ * @return The status value.
+ */
+function readSecureStatusFile() {
+  let file = getSecureOutputFile("status");
+  if (!file.exists()) {
+    debugDump("update status file does not exist, path: " + file.path);
+    return STATE_NONE;
+  }
+  return readFile(file).split("\n")[0];
+}
+
+/**
+ * Get the nsIFile in the secure update log directory. The file name is always
+ * the value of gTestID with either a file extension of 'log' or 'status'.
+ *
+ * @param  aFileExt
+ *         The file extension.
+ * @return The nsIFile of the secure update file.
+ */
+function getSecureOutputFile(aFileExt) {
+  let file = getMaintSvcDir();
+  file.append("UpdateLogs");
+  file.append(gTestID + "." + aFileExt);
+  return file;
+}
+
 /**
  * Get the nsIFile for a Windows special folder determined by the CSIDL
  * passed.
@@ -1967,7 +2021,7 @@ function runUpdate(
   let status = readStatusFile();
   if (
     (!gIsServiceTest && process.exitValue != aExpectedExitValue) ||
-    status != aExpectedStatus
+    (status != aExpectedStatus && !gIsServiceTest && !isInvalidArgTest)
   ) {
     if (process.exitValue != aExpectedExitValue) {
       logTestInfo(
@@ -1988,6 +2042,13 @@ function runUpdate(
     logUpdateLog(FILE_LAST_UPDATE_LOG);
   }
 
+  if (gIsServiceTest && isInvalidArgTest) {
+    let secureStatus = readSecureStatusFile();
+    if (secureStatus != STATE_NONE) {
+      status = secureStatus;
+    }
+  }
+
   if (!gIsServiceTest) {
     Assert.equal(
       process.exitValue,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgInstallDirPathTooLongFailure.js

@@ -31,13 +31,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_INSTALL_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgInstallDirPathTraversalFailure.js

@@ -28,13 +28,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_INSTALL_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgInstallWorkingDirPathNotSameFailure_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_APPLYTO_DIR_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgStageDirNotInInstallDirFailure_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_APPLYTO_DIR_STAGED_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgWorkingDirPathLocalUNCFailure_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_WORKING_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_base_updater/invalidArgWorkingDirPathRelativeFailure.js

@@ -22,13 +22,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_WORKING_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgInstallDirPathTooLongFailureSvc.js

@@ -31,13 +31,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_INSTALL_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgInstallDirPathTraversalFailureSvc.js

@@ -28,13 +28,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_INSTALL_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgInstallWorkingDirPathNotSameFailureSvc_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_APPLYTO_DIR_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgStageDirNotInInstallDirFailureSvc_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_APPLYTO_DIR_STAGED_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgWorkingDirPathLocalUNCFailureSvc_win.js

@@ -23,13 +23,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_WORKING_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/tests/unit_service_updater/invalidArgWorkingDirPathRelativeFailureSvc.js

@@ -22,13 +22,14 @@ async function run_test() {
   checkFilesAfterUpdateFailure(getApplyDirFile);
   await waitForUpdateXMLFiles();
   if (gIsServiceTest) {
-    checkUpdateManager(
-      STATE_NONE,
-      false,
-      STATE_FAILED,
-      SERVICE_INVALID_WORKING_DIR_PATH_ERROR,
-      1
-    );
+    // The invalid argument service tests launch the maintenance service
+    // directly so the unelevated updater doesn't handle the invalid argument.
+    // By doing this it is possible to test that the maintenance service
+    // properly handles the invalid argument but since the updater isn't used to
+    // launch the maintenance service the update.status file isn't copied from
+    // the secure log directory to the patch directory and the update manager
+    // won't read the failure from the update.status file.
+    checkUpdateManager(STATE_NONE, false, STATE_PENDING_SVC, 0, 1);
   } else {
     checkUpdateManager(
       STATE_NONE,

toolkit/mozapps/update/updater/updater.cpp

@@ -129,7 +129,6 @@ BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer, LPCWSTR siblingFilePath,
         CloseHandle(handle);                        \
       }                                             \
       if (NS_tremove(path) && errno != ENOENT) {    \
-        LogFinish();                                \
         return retCode;                             \
       }                                             \
     }
@@ -283,6 +282,12 @@ static bool sUsingService = false;
 static NS_tchar gCallbackRelPath[MAXPATHLEN];
 static NS_tchar gCallbackBackupPath[MAXPATHLEN];
 static NS_tchar gDeleteDirPath[MAXPATHLEN];
+
+// Whether to copy the update.log and update.status file to the update patch
+// directory from a secure directory.
+static bool gCopyOutputFiles = false;
+// Whether to write the update.log and update.status file to a secure directory.
+static bool gUseSecureOutputPath = false;
 #endif
 
 static const NS_tchar kWhitespace[] = NS_T(" \t");
@@ -343,6 +348,71 @@ static bool EnvHasValue(const char* name) {
 }
 #endif
 
+#ifdef XP_WIN
+/**
+ * Obtains the update ID from the secure id file located in secure output
+ * directory.
+ *
+ * @param  outBuf
+ *         A buffer of size UUID_LEN (e.g. 37) to store the result. The uuid is
+ *         36 characters in length and 1 more for null termination.
+ * @return true if successful
+ */
+bool GetSecureID(char* outBuf) {
+  NS_tchar idFilePath[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFilePath(gPatchDirPath, L".id", idFilePath)) {
+    return false;
+  }
+
+  AutoFile idFile(NS_tfopen(idFilePath, NS_T("rb")));
+  if (idFile == nullptr) {
+    return false;
+  }
+
+  size_t read = fread(outBuf, UUID_LEN - 1, 1, idFile);
+  if (read != 1) {
+    return false;
+  }
+
+  outBuf[UUID_LEN] = '\0';
+  return true;
+}
+#endif
+
+/**
+ * Calls LogFinish for the update log. On Windows, the unelevated updater copies
+ * the update status file and the update log file that were written by the
+ * elevated updater from the secure directory to the update patch directory.
+ *
+ * NOTE: All calls to WriteStatusFile MUST happen before calling output_finish
+ *       because this function copies the update status file for the elevated
+ *       updater and writing the status file after calling output_finish will
+ *       overwrite it.
+ */
+static void output_finish() {
+  LogFinish();
+#ifdef XP_WIN
+  if (gCopyOutputFiles) {
+    NS_tchar srcStatusPath[MAXPATHLEN + 1] = {NS_T('\0')};
+    if (GetSecureOutputFilePath(gPatchDirPath, L".status", srcStatusPath)) {
+      NS_tchar dstStatusPath[MAXPATHLEN + 1] = {NS_T('\0')};
+      NS_tsnprintf(dstStatusPath,
+                   sizeof(dstStatusPath) / sizeof(dstStatusPath[0]),
+                   NS_T("%s\\update.status"), gPatchDirPath);
+      CopyFileW(srcStatusPath, dstStatusPath, false);
+    }
+
+    NS_tchar srcLogPath[MAXPATHLEN + 1] = {NS_T('\0')};
+    if (GetSecureOutputFilePath(gPatchDirPath, L".log", srcLogPath)) {
+      NS_tchar dstLogPath[MAXPATHLEN + 1] = {NS_T('\0')};
+      NS_tsnprintf(dstLogPath, sizeof(dstLogPath) / sizeof(dstLogPath[0]),
+                   NS_T("%s\\update.log"), gPatchDirPath);
+      CopyFileW(srcLogPath, dstLogPath, false);
+    }
+  }
+#endif
+}
+
 /**
  * Coverts a relative update path to a full path.
  *
@@ -1958,9 +2028,15 @@ bool LaunchWinPostProcess(const WCHAR* installationDir,
   }
 
   WCHAR slogFile[MAX_PATH + 1] = {L'\0'};
-  wcsncpy(slogFile, updateInfoDir, MAX_PATH);
-  if (!PathAppendSafe(slogFile, L"update.log")) {
-    return false;
+  if (gCopyOutputFiles) {
+    if (!GetSecureOutputFilePath(gPatchDirPath, L".log", slogFile)) {
+      return false;
+    }
+  } else {
+    wcsncpy(slogFile, updateInfoDir, MAX_PATH);
+    if (!PathAppendSafe(slogFile, L"update.log")) {
+      return false;
+    }
   }
 
   WCHAR dummyArg[14] = {L'\0'};
@@ -2042,52 +2118,78 @@ static void LaunchCallbackApp(const NS_tchar* workingDir, int argc,
 }
 
 static bool WriteToFile(const NS_tchar* aFilename, const char* aStatus) {
-  NS_tchar filename[MAXPATHLEN] = {NS_T('\0')};
+  NS_tchar statusFilePath[MAXPATHLEN + 1] = {NS_T('\0')};
 #if defined(XP_WIN)
-  // The temp file is not removed on failure since there is client code that
-  // will remove it.
-  if (!GetUUIDTempFilePath(gPatchDirPath, L"sta", filename)) {
-    return false;
+  if (gUseSecureOutputPath) {
+    if (!GetSecureOutputFilePath(gPatchDirPath, L".status", statusFilePath)) {
+      return false;
+    }
+  } else {
+    NS_tsnprintf(statusFilePath,
+                 sizeof(statusFilePath) / sizeof(statusFilePath[0]),
+                 NS_T("%s\\%s"), gPatchDirPath, aFilename);
   }
 #else
-  NS_tsnprintf(filename, sizeof(filename) / sizeof(filename[0]), NS_T("%s/%s"),
-               gPatchDirPath, aFilename);
+  NS_tsnprintf(statusFilePath,
+               sizeof(statusFilePath) / sizeof(statusFilePath[0]),
+               NS_T("%s/%s"), gPatchDirPath, aFilename);
+  // Make sure that the directory for the update status file exists
+  if (ensure_parent_dir(statusFilePath)) {
+    return false;
+  }
 #endif
 
-  // Make sure that the directory for the update status file exists
-  if (ensure_parent_dir(filename)) {
+  AutoFile statusFile(NS_tfopen(statusFilePath, NS_T("wb+")));
+  if (statusFile == nullptr) {
     return false;
   }
 
-  // This is scoped to make the AutoFile close the file so it is possible to
-  // move the temp file to the update.status file on Windows.
-  {
-    AutoFile file(NS_tfopen(filename, NS_T("wb+")));
-    if (file == nullptr) {
-      return false;
-    }
-
-    if (fwrite(aStatus, strlen(aStatus), 1, file) != 1) {
-      return false;
-    }
+  if (fwrite(aStatus, strlen(aStatus), 1, statusFile) != 1) {
+    return false;
   }
 
 #if defined(XP_WIN)
-  NS_tchar dstfilename[MAXPATHLEN] = {NS_T('\0')};
-  NS_tsnprintf(dstfilename, sizeof(dstfilename) / sizeof(dstfilename[0]),
-               NS_T("%s\\%s"), gPatchDirPath, aFilename);
-  if (MoveFileExW(filename, dstfilename, MOVEFILE_REPLACE_EXISTING) == 0) {
-    return false;
+  if (gUseSecureOutputPath) {
+    // This is done after the update status file has been written so if the
+    // write to the update status file fails an existing update status file
+    // won't be used.
+    if (!WriteSecureIDFile(gPatchDirPath)) {
+      return false;
+    }
   }
 #endif
 
   return true;
 }
 
+/**
+ * Writes a string to the update.status file.
+ *
+ * NOTE: All calls to WriteStatusFile MUST happen before calling output_finish
+ *       because the output_finish function copies the update status file for
+ *       the elevated updater and writing the status file after calling
+ *       output_finish will overwrite it.
+ *
+ * @param  aStatus
+ *         The string to write to the update.status file.
+ * @return true on success.
+ */
 static bool WriteStatusFile(const char* aStatus) {
   return WriteToFile(NS_T("update.status"), aStatus);
 }
 
+/**
+ * Writes a string to the update.status file based on the status param.
+ *
+ * NOTE: All calls to WriteStatusFile MUST happen before calling output_finish
+ *       because the output_finish function copies the update status file for
+ *       the elevated updater and writing the status file after calling
+ *       output_finish will overwrite it.
+ *
+ * @param  status
+ *         A status code used to determine what string to write to the
+ *         update.status file (see code).
+ */
 static void WriteStatusFile(int status) {
   const char* text;
 
@@ -2139,20 +2241,21 @@ static bool IsUpdateStatusPendingService() {
 
 #ifdef XP_WIN
 /*
- * Read the update.status file and sets isSuccess to true if
- * the status is set to succeeded.
+ * Reads the secure update status file and sets isSucceeded to true if the
+ * status is set to succeeded.
  *
  * @param  isSucceeded Out parameter for specifying if the status
  *         is set to succeeded or not.
  * @return true if the information was retrieved and it is succeeded.
  */
-static bool IsUpdateStatusSucceeded(bool& isSucceeded) {
+static bool IsSecureUpdateStatusSucceeded(bool& isSucceeded) {
   isSucceeded = false;
-  NS_tchar filename[MAXPATHLEN];
-  NS_tsnprintf(filename, sizeof(filename) / sizeof(filename[0]),
-               NS_T("%s/update.status"), gPatchDirPath);
+  NS_tchar statusFilePath[MAX_PATH + 1] = {L'\0'};
+  if (!GetSecureOutputFilePath(gPatchDirPath, L".status", statusFilePath)) {
+    return FALSE;
+  }
 
-  AutoFile file(NS_tfopen(filename, NS_T("rb")));
+  AutoFile file(NS_tfopen(statusFilePath, NS_T("rb")));
   if (file == nullptr) {
     return false;
   }
@@ -2687,6 +2790,15 @@ int NS_main(int argc, NS_tchar** argv) {
   NS_tstrncpy(gPatchDirPath, argv[1], MAXPATHLEN);
   gPatchDirPath[MAXPATHLEN - 1] = NS_T('\0');
 
+#ifdef XP_WIN
+  NS_tchar elevatedLockFilePath[MAXPATHLEN] = {NS_T('\0')};
+  NS_tsnprintf(elevatedLockFilePath,
+               sizeof(elevatedLockFilePath) / sizeof(elevatedLockFilePath[0]),
+               NS_T("%s\\update_elevated.lock"), gPatchDirPath);
+  gUseSecureOutputPath =
+      sUsingService || (NS_tremove(elevatedLockFilePath) && errno != ENOENT);
+#endif
+
   // This check is also performed in workmonitor.cpp since the maintenance
   // service can be called directly.
   if (!IsValidFullPath(argv[2])) {
@@ -2851,7 +2963,23 @@ int NS_main(int argc, NS_tchar** argv) {
   }
 #endif
 
-  LogInit(gPatchDirPath, NS_T("update.log"));
+  NS_tchar logFilePath[MAXPATHLEN + 1] = {L'\0'};
+#ifdef XP_WIN
+  if (gUseSecureOutputPath) {
+    // Remove the secure output files so it is easier to determine when new
+    // files are created in the unelevated updater.
+    RemoveSecureOutputFiles(gPatchDirPath);
+
+    (void)GetSecureOutputFilePath(gPatchDirPath, L".log", logFilePath);
+  } else {
+    NS_tsnprintf(logFilePath, sizeof(logFilePath) / sizeof(logFilePath[0]),
+                 NS_T("%s\\update.log"), gPatchDirPath);
+  }
+#else
+  NS_tsnprintf(logFilePath, sizeof(logFilePath) / sizeof(logFilePath[0]),
+               NS_T("%s/update.log"), gPatchDirPath);
+#endif
+  LogInit(logFilePath);
 
   if (!WriteStatusFile("applying")) {
     LOG(("failed setting status to 'applying'"));
@@ -2861,6 +2989,7 @@ int NS_main(int argc, NS_tchar** argv) {
       CleanupElevatedMacUpdate(true);
     }
 #endif
+    output_finish();
     return 1;
   }
 
@@ -2883,7 +3012,7 @@ int NS_main(int argc, NS_tchar** argv) {
       LOG(
           ("Installation directory and working directory must be the same "
            "for non-staged updates. Exiting."));
-      LogFinish();
+      output_finish();
       return 1;
     }
 
@@ -2894,7 +3023,7 @@ int NS_main(int argc, NS_tchar** argv) {
     if (!PathRemoveFileSpecW(workingDirParent)) {
       WriteStatusFile(REMOVE_FILE_SPEC_ERROR);
       LOG(("Error calling PathRemoveFileSpecW: %d", GetLastError()));
-      LogFinish();
+      output_finish();
       return 1;
     }
 
@@ -2903,7 +3032,7 @@ int NS_main(int argc, NS_tchar** argv) {
       LOG(
           ("The apply-to directory must be the same as or "
            "a child of the installation directory! Exiting."));
-      LogFinish();
+      output_finish();
       return 1;
     }
   }
@@ -2952,11 +3081,10 @@ int NS_main(int argc, NS_tchar** argv) {
   // we will instead fallback to not using the service and display a UAC prompt.
   int lastFallbackError = FALLBACKKEY_UNKNOWN_ERROR;
 
-  // Launch a second instance of the updater with the runas verb on Windows
-  // when write access is denied to the installation directory and the update
-  // isn't being staged.
+  // Check whether a second instance of the updater should be launched by the
+  // maintenance service or with the 'runas' verb when write access is denied to
+  // the installation directory and the update isn't being staged.
   HANDLE updateLockFileHandle = INVALID_HANDLE_VALUE;
-  NS_tchar elevatedLockFilePath[MAXPATHLEN] = {NS_T('\0')};
   if (!sUsingService &&
       (argc > callbackIndex || sStagedUpdate || sReplaceRequest)) {
     NS_tchar updateLockFilePath[MAXPATHLEN];
@@ -2998,6 +3126,7 @@ int NS_main(int argc, NS_tchar** argv) {
         WriteStatusFile(DELETE_ERROR_STAGING_LOCK_FILE);
       }
       LOG(("Update already in progress! Exiting"));
+      output_finish();
       return 1;
     }
 
@@ -3005,10 +3134,6 @@ int NS_main(int argc, NS_tchar** argv) {
         CreateFileW(updateLockFilePath, GENERIC_READ | GENERIC_WRITE, 0,
                     nullptr, OPEN_ALWAYS, FILE_FLAG_DELETE_ON_CLOSE, nullptr);
 
-    NS_tsnprintf(elevatedLockFilePath,
-                 sizeof(elevatedLockFilePath) / sizeof(elevatedLockFilePath[0]),
-                 NS_T("%s/update_elevated.lock"), gPatchDirPath);
-
     // Even if a file has no sharing access, you can still get its attributes
     bool startedFromUnelevatedUpdater =
         GetFileAttributesW(elevatedLockFilePath) != INVALID_FILE_ATTRIBUTES;
@@ -3029,7 +3154,8 @@ int NS_main(int argc, NS_tchar** argv) {
         (useService && testOnlyFallbackKeyExists && noServiceFallback)) {
       HANDLE elevatedFileHandle;
       if (NS_tremove(elevatedLockFilePath) && errno != ENOENT) {
-        fprintf(stderr, "Unable to create elevated lock file! Exiting\n");
+        LOG(("Unable to create elevated lock file! Exiting"));
+        output_finish();
         return 1;
       }
 
@@ -3038,12 +3164,14 @@ int NS_main(int argc, NS_tchar** argv) {
                       nullptr, OPEN_ALWAYS, FILE_FLAG_DELETE_ON_CLOSE, nullptr);
       if (elevatedFileHandle == INVALID_HANDLE_VALUE) {
         LOG(("Unable to create elevated lock file! Exiting"));
+        output_finish();
         return 1;
       }
 
       auto cmdLine = mozilla::MakeCommandLine(argc - 1, argv + 1);
       if (!cmdLine) {
         CloseHandle(elevatedFileHandle);
+        output_finish();
         return 1;
       }
 
@@ -3102,6 +3230,11 @@ int NS_main(int argc, NS_tchar** argv) {
       // If we still want to use the service try to launch the service
       // comamnd for the update.
       if (useService) {
+        // Get the secure ID before trying to update so it is possible to
+        // determine if the updater or the maintenance service has created a
+        // new one.
+        char uuidStringBefore[UUID_LEN] = {'\0'};
+        bool checkID = GetSecureID(uuidStringBefore);
         // Write a catchall service failure status in case it fails without
         // changing the status.
         WriteStatusFile(SERVICE_UPDATE_STATUS_UNCHANGED);
@@ -3139,6 +3272,18 @@ int NS_main(int argc, NS_tchar** argv) {
             // something seriously wrong.
             lastFallbackError = FALLBACKKEY_SERVICE_NO_STOP_ERROR;
             useService = false;
+          } else {
+            // Copy the secure output files if the secure ID has changed.
+            gCopyOutputFiles = true;
+            char uuidStringAfter[UUID_LEN] = {'\0'};
+            if (checkID && GetSecureID(uuidStringAfter) &&
+                strncmp(uuidStringBefore, uuidStringAfter,
+                        sizeof(uuidStringBefore)) == 0) {
+              LOG(
+                  ("The secure ID hasn't changed after launching the updater "
+                   "using the service"));
+              gCopyOutputFiles = false;
+            }
           }
         } else {
           lastFallbackError = FALLBACKKEY_LAUNCH_ERROR;
@@ -3156,19 +3301,19 @@ int NS_main(int argc, NS_tchar** argv) {
         LOG(
             ("Non-critical update staging error! Falling back to non-staged "
              "updates and exiting"));
+        output_finish();
         return 0;
       }
 
-      // If we started the service command, and it finished, check the
-      // update.status file to make sure it succeeded, and if it did
-      // we need to manually start the PostUpdate process from the
-      // current user's session of this unelevated updater.exe the
-      // current process is running as.
-      // Note that we don't need to do this if we're just staging the update,
-      // as the PostUpdate step runs when performing the replacing in that case.
+      // If we started the service command, and it finished, check the secure
+      // update status file to make sure that it succeeded, and if it did we
+      // need to launch the PostUpdate process in the unelevated updater which
+      // is running in the current user's session. Note that we don't need to do
+      // this when staging an update since the PostUpdate step runs during the
+      // replace request.
       if (useService && !sStagedUpdate) {
         bool updateStatusSucceeded = false;
-        if (IsUpdateStatusSucceeded(updateStatusSucceeded) &&
+        if (IsSecureUpdateStatusSucceeded(updateStatusSucceeded) &&
             updateStatusSucceeded) {
           if (!LaunchWinPostProcess(gInstallDirPath, gPatchDirPath)) {
             fprintf(stderr,
@@ -3186,6 +3331,14 @@ int NS_main(int argc, NS_tchar** argv) {
       // using the service is because we are testing.
       if (!useService && !noServiceFallback &&
           updateLockFileHandle == INVALID_HANDLE_VALUE) {
+        // Get the secure ID before trying to update so it is possible to
+        // determine if the updater has created a new one.
+        char uuidStringBefore[UUID_LEN] = {'\0'};
+        bool checkID = GetSecureID(uuidStringBefore);
+        // Write a catchall failure status in case it fails without changing the
+        // status.
+        WriteStatusFile(UPDATE_STATUS_UNCHANGED);
+
         SHELLEXECUTEINFO sinfo;
         memset(&sinfo, 0, sizeof(SHELLEXECUTEINFO));
         sinfo.cbSize = sizeof(SHELLEXECUTEINFO);
@@ -3202,11 +3355,34 @@ int NS_main(int argc, NS_tchar** argv) {
         if (result) {
           WaitForSingleObject(sinfo.hProcess, INFINITE);
           CloseHandle(sinfo.hProcess);
+
+          // Copy the secure output files if the secure ID has changed.
+          gCopyOutputFiles = true;
+          char uuidStringAfter[UUID_LEN] = {'\0'};
+          if (checkID && GetSecureID(uuidStringAfter) &&
+              strncmp(uuidStringBefore, uuidStringAfter,
+                      sizeof(uuidStringBefore)) == 0) {
+            LOG(
+                ("The secure ID hasn't changed after launching the updater "
+                 "using runas"));
+            gCopyOutputFiles = false;
+          }
         } else {
+          // Don't copy the secure output files if the elevation request was
+          // canceled since the status file written below is in the patch
+          // directory. At this point it should already be set to false and this
+          // is set here to make it clear that it should be false at this point
+          // and to prevent future changes from regressing this code.
+          gCopyOutputFiles = false;
           WriteStatusFile(ELEVATION_CANCELED);
         }
       }
 
+      // Note: The PostUpdate process is launched by the elevated updater which
+      // is running in the current user's session when the update is successful
+      // and doesn't need to be performed by the unelevated updater as is done
+      // when the maintenance service launches the updater.
+
       if (argc > callbackIndex) {
         LaunchCallbackApp(argv[5], argc - callbackIndex, argv + callbackIndex,
                           sUsingService);
@@ -3219,6 +3395,7 @@ int NS_main(int argc, NS_tchar** argv) {
         // We didn't use the service and we did run the elevated updater.exe.
         // The elevated updater.exe is responsible for writing out the
         // update.status file.
+        output_finish();
         return 0;
       } else if (useService) {
         // The service command was launched. The service is responsible for
@@ -3226,6 +3403,7 @@ int NS_main(int argc, NS_tchar** argv) {
         if (updateLockFileHandle != INVALID_HANDLE_VALUE) {
           CloseHandle(updateLockFileHandle);
         }
+        output_finish();
         return 0;
       } else {
         // Otherwise the service command was not launched at all.
@@ -3235,10 +3413,20 @@ int NS_main(int argc, NS_tchar** argv) {
         // We only currently use this env var from XPCShell tests.
         CloseHandle(updateLockFileHandle);
         WriteStatusFile(lastFallbackError);
+        output_finish();
         return 0;
       }
+      // This is the end of the code block for launching another instance of the
+      // updater using either the maintenance service or with the 'runas' verb
+      // when the updater doesn't have write access to the installation
+      // directory.
     }
+    // This is the end of the code block when the updater was not launched by
+    // the service that checks whether the updater has write access to the
+    // installation directory.
   }
+  // If we made it this far this is the updater instance that will perform the
+  // actual update and gCopyOutputFiles will be false (e.g. the default value).
 #endif
 
   if (sStagedUpdate) {
@@ -3256,6 +3444,7 @@ int NS_main(int argc, NS_tchar** argv) {
       // WRITE_ERROR is one of the cases where the staging failure falls back to
       // applying the update on startup.
       WriteStatusFile(WRITE_ERROR);
+      output_finish();
       return 0;
     }
 #endif
@@ -3273,6 +3462,7 @@ int NS_main(int argc, NS_tchar** argv) {
         CleanupElevatedMacUpdate(true);
       }
 #endif
+      output_finish();
       return 1;
     }
   }
@@ -3282,9 +3472,9 @@ int NS_main(int argc, NS_tchar** argv) {
   if (!GetLongPathNameW(
           gWorkingDirPath, applyDirLongPath,
           sizeof(applyDirLongPath) / sizeof(applyDirLongPath[0]))) {
-    LOG(("NS_main: unable to find apply to dir: " LOG_S, gWorkingDirPath));
-    LogFinish();
     WriteStatusFile(WRITE_ERROR_APPLY_DIR_PATH);
+    LOG(("NS_main: unable to find apply to dir: " LOG_S, gWorkingDirPath));
+    output_finish();
     EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
     if (argc > callbackIndex) {
       LaunchCallbackApp(argv[5], argc - callbackIndex, argv + callbackIndex,
@@ -3336,9 +3526,9 @@ int NS_main(int argc, NS_tchar** argv) {
     if (!GetLongPathNameW(
             targetPath, callbackLongPath,
             sizeof(callbackLongPath) / sizeof(callbackLongPath[0]))) {
-      LOG(("NS_main: unable to find callback file: " LOG_S, targetPath));
-      LogFinish();
       WriteStatusFile(WRITE_ERROR_CALLBACK_PATH);
+      LOG(("NS_main: unable to find callback file: " LOG_S, targetPath));
+      output_finish();
       EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
       if (argc > callbackIndex) {
         LaunchCallbackApp(argv[5], argc - callbackIndex, argv + callbackIndex,
@@ -3382,9 +3572,9 @@ int NS_main(int argc, NS_tchar** argv) {
       if (callbackBackupPathLen < 0 ||
           callbackBackupPathLen >=
               static_cast<int>(callbackBackupPathBufSize)) {
-        LOG(("NS_main: callback backup path truncated"));
-        LogFinish();
         WriteStatusFile(USAGE_ERROR);
+        LOG(("NS_main: callback backup path truncated"));
+        output_finish();
 
         // Don't attempt to launch the callback when the callback path is
         // longer than expected.
@@ -3395,16 +3585,15 @@ int NS_main(int argc, NS_tchar** argv) {
       // Make a copy of the callback executable so it can be read when patching.
       if (!CopyFileW(argv[callbackIndex], gCallbackBackupPath, false)) {
         DWORD copyFileError = GetLastError();
-        LOG(("NS_main: failed to copy callback file " LOG_S
-             " into place at " LOG_S,
-             argv[callbackIndex], gCallbackBackupPath));
-        LogFinish();
         if (copyFileError == ERROR_ACCESS_DENIED) {
           WriteStatusFile(WRITE_ERROR_ACCESS_DENIED);
         } else {
           WriteStatusFile(WRITE_ERROR_CALLBACK_APP);
         }
-
+        LOG(("NS_main: failed to copy callback file " LOG_S
+             " into place at " LOG_S,
+             argv[callbackIndex], gCallbackBackupPath));
+        output_finish();
         EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
         LaunchCallbackApp(argv[callbackIndex], argc - callbackIndex,
                           argv + callbackIndex, sUsingService);
@@ -3447,7 +3636,6 @@ int NS_main(int argc, NS_tchar** argv) {
               ("NS_main: callback app file in use, failed to exclusively open "
                "executable file: " LOG_S,
                argv[callbackIndex]));
-          LogFinish();
           if (lastWriteError == ERROR_ACCESS_DENIED) {
             WriteStatusFile(WRITE_ERROR_ACCESS_DENIED);
           } else {
@@ -3460,6 +3648,7 @@ int NS_main(int argc, NS_tchar** argv) {
                  "path: " LOG_S,
                  gCallbackBackupPath));
           }
+          output_finish();
           EXIT_WHEN_ELEVATED(elevatedLockFilePath, updateLockFileHandle, 1);
           LaunchCallbackApp(argv[5], argc - callbackIndex, argv + callbackIndex,
                             sUsingService);
@@ -3562,7 +3751,7 @@ int NS_main(int argc, NS_tchar** argv) {
   }
 #endif /* XP_MACOSX */
 
-  LogFinish();
+  output_finish();
 
   int retVal = LaunchCallbackAndPostProcessApps(argc, argv, callbackIndex
 #ifdef XP_WIN