Bug 1247239 - Allow the maintenance service to copy updater.exe only from the installation directory, and verify it first; r=rstrong
authorMatt Howell <mhowell@mozilla.com>
Fri, 08 Jul 2016 10:58:21 -0700
changeset 330674 6cca6d362e38a4eb1f680c62a17137e04b1169f2
parent 330673 fd7eaf008b62b564dcb6e31aa76ba5c476de559d
child 330675 c686dd1366a3c3e45a64e71fec65cd587be38dac
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs1247239
milestone50.0a1
Bug 1247239 - Allow the maintenance service to copy updater.exe only from the installation directory, and verify it first; r=rstrong MozReview-Commit-ID: 68tbdamvFf0
toolkit/components/maintenanceservice/workmonitor.cpp
--- a/toolkit/components/maintenanceservice/workmonitor.cpp
+++ b/toolkit/components/maintenanceservice/workmonitor.cpp
@@ -25,17 +25,17 @@
 #include "errors.h"
 
 // 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.
 static const int TIME_TO_WAIT_ON_UPDATER = 15 * 60 * 1000;
 wchar_t* MakeCommandLine(int argc, wchar_t** argv);
 BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode);
-BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer,  LPCWSTR siblingFilePath, 
+BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer,  LPCWSTR siblingFilePath,
                             LPCWSTR newFileName);
 
 /*
  * Read the 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  isApplying Out parameter for specifying if the status
@@ -283,16 +283,127 @@ StartUpdateProcess(int argc,
   // Empty value on putenv is how you remove an env variable in Windows
   putenv(const_cast<char*>("MOZ_USING_SERVICE="));
 
   free(cmdLine);
   return updateWasSuccessful;
 }
 
 /**
+ * Validates a file as an official updater.
+ *
+ * @param updater     Path to the updater to validate
+ * @param installDir  Path to the application installation
+ *                    being updated
+ * @param updateDir   Update applyTo direcotry,
+ *                    where logs will be written
+ *
+ * @return true if updater is the path to a valid updater
+ */
+static bool
+UpdaterIsValid(LPWSTR updater, LPWSTR installDir, LPWSTR updateDir)
+{
+  // Make sure the path to the updater to use for the update is local.
+  // We do this check to make sure that file locking is available for
+  // race condition security checks.
+  BOOL isLocal = FALSE;
+  if (!IsLocalFile(updater, isLocal) || !isLocal) {
+    LOG_WARN(("Filesystem in path %ls is not supported (%d)",
+              updater, GetLastError()));
+    if (!WriteStatusFailure(updateDir, SERVICE_UPDATER_NOT_FIXED_DRIVE)) {
+      LOG_WARN(("Could not write update.status service update failure.  (%d)",
+                GetLastError()));
+    }
+    return false;
+  }
+
+  nsAutoHandle noWriteLock(CreateFileW(updater, GENERIC_READ, FILE_SHARE_READ,
+                                       nullptr, OPEN_EXISTING, 0, nullptr));
+  if (INVALID_HANDLE_VALUE == noWriteLock) {
+    LOG_WARN(("Could not set no write sharing access on file.  (%d)",
+              GetLastError()));
+    if (!WriteStatusFailure(updateDir, SERVICE_COULD_NOT_LOCK_UPDATER)) {
+      LOG_WARN(("Could not write update.status service update failure.  (%d)",
+                GetLastError()));
+    }
+    return false;
+  }
+
+  // Verify that the updater.exe that we are executing is the same
+  // as the one in the installation directory which we are updating.
+  // The installation dir that we are installing to is installDir.
+  WCHAR installDirUpdater[MAX_PATH + 1] = { L'\0' };
+  wcsncpy(installDirUpdater, installDir, MAX_PATH);
+  if (!PathAppendSafe(installDirUpdater, L"updater.exe")) {
+    LOG_WARN(("Install directory updater could not be determined."));
+    return false;
+  }
+
+  BOOL updaterIsCorrect;
+  if (!VerifySameFiles(updater, installDirUpdater, updaterIsCorrect)) {
+    LOG_WARN(("Error checking if the updaters are the same.\n"
+              "Path 1: %ls\nPath 2: %ls", updater, installDirUpdater));
+    return false;
+  }
+
+  if (!updaterIsCorrect) {
+    LOG_WARN(("The updaters do not match, updater will not run.\n"
+              "Path 1: %ls\nPath 2: %ls", updater, installDirUpdater));
+    if (!WriteStatusFailure(updateDir, SERVICE_UPDATER_COMPARE_ERROR)) {
+      LOG_WARN(("Could not write update.status updater compare failure."));
+    }
+    return false;
+  }
+
+  LOG(("updater.exe was compared successfully to the installation directory"
+       " updater.exe."));
+
+  // Check to make sure the updater.exe module has the unique updater identity.
+  // This is a security measure to make sure that the signed executable that
+  // we will run is actually an updater.
+  bool result = true;
+  HMODULE updaterModule = LoadLibraryEx(updater, nullptr,
+                                        LOAD_LIBRARY_AS_DATAFILE);
+  if (!updaterModule) {
+    LOG_WARN(("updater.exe module could not be loaded. (%d)", GetLastError()));
+    result = false;
+  } else {
+    char updaterIdentity[64];
+    if (!LoadStringA(updaterModule, IDS_UPDATER_IDENTITY,
+      updaterIdentity, sizeof(updaterIdentity))) {
+      LOG_WARN(("The updater.exe application does not contain the Mozilla"
+                " updater identity."));
+      result = false;
+    }
+
+    if (strcmp(updaterIdentity, UPDATER_IDENTITY_STRING)) {
+      LOG_WARN(("The updater.exe identity string is not valid."));
+      result = false;
+    }
+    FreeLibrary(updaterModule);
+  }
+
+  if (result) {
+    LOG(("The updater.exe application contains the Mozilla"
+         " updater identity."));
+  } else {
+    if (!WriteStatusFailure(updateDir, SERVICE_UPDATER_IDENTITY_ERROR)) {
+      LOG_WARN(("Could not write update.status no updater identity."));
+    }
+    return false;
+  }
+
+#ifndef DISABLE_UPDATER_AUTHENTICODE_CHECK
+  return DoesBinaryMatchAllowedCertificates(installDir, updater);
+#else
+  return true;
+#endif
+}
+
+/**
  * Processes a software update command
  *
  * @param  argc           The number of arguments in argv
  * @param  argv           The arguments normally passed to updater.exe
  *                        argv[0] must be the path to updater.exe
  * @return TRUE if the update was successful.
  */
 BOOL
@@ -319,123 +430,17 @@ ProcessSoftwareUpdateCommand(DWORD argc,
     LOG_WARN(("Could not get the installation directory"));
     if (!WriteStatusFailure(argv[1],
                             SERVICE_INSTALLDIR_ERROR)) {
       LOG_WARN(("Could not write update.status for GetInstallationDir failure."));
     }
     return FALSE;
   }
 
-  // Make sure the path to the updater to use for the update is local.
-  // We do this check to make sure that file locking is available for
-  // race condition security checks.
-  BOOL isLocal = FALSE;
-  if (!IsLocalFile(argv[0], isLocal) || !isLocal) {
-    LOG_WARN(("Filesystem in path %ls is not supported (%d)",
-              argv[0], GetLastError()));
-    if (!WriteStatusFailure(argv[1],
-                            SERVICE_UPDATER_NOT_FIXED_DRIVE)) {
-      LOG_WARN(("Could not write update.status service update failure.  (%d)",
-                GetLastError()));
-    }
-    return FALSE;
-  }
-
-  nsAutoHandle noWriteLock(CreateFileW(argv[0], GENERIC_READ, FILE_SHARE_READ,
-                                       nullptr, OPEN_EXISTING, 0, nullptr));
-  if (INVALID_HANDLE_VALUE == noWriteLock) {
-      LOG_WARN(("Could not set no write sharing access on file.  (%d)",
-                GetLastError()));
-    if (!WriteStatusFailure(argv[1],
-                            SERVICE_COULD_NOT_LOCK_UPDATER)) {
-      LOG_WARN(("Could not write update.status service update failure.  (%d)",
-                GetLastError()));
-    }
-    return FALSE;
-  }
-
-  // Verify that the updater.exe that we are executing is the same
-  // as the one in the installation directory which we are updating.
-  // The installation dir that we are installing to is installDir.
-  WCHAR installDirUpdater[MAX_PATH + 1] = { L'\0' };
-  wcsncpy(installDirUpdater, installDir, MAX_PATH);
-  if (!PathAppendSafe(installDirUpdater, L"updater.exe")) {
-    LOG_WARN(("Install directory updater could not be determined."));
-    result = FALSE;
-  }
-
-  BOOL updaterIsCorrect;
-  if (result && !VerifySameFiles(argv[0], installDirUpdater,
-                                 updaterIsCorrect)) {
-    LOG_WARN(("Error checking if the updaters are the same.\n"
-              "Path 1: %ls\nPath 2: %ls", argv[0], installDirUpdater));
-    result = FALSE;
-  }
-
-  if (result && !updaterIsCorrect) {
-    LOG_WARN(("The updaters do not match, updater will not run.\n"
-              "Path 1: %ls\nPath 2: %ls", argv[0], installDirUpdater));
-    result = FALSE;
-  }
-
-  if (result) {
-    LOG(("updater.exe was compared successfully to the installation directory"
-         " updater.exe."));
-  } else {
-    if (!WriteStatusFailure(argv[1],
-                            SERVICE_UPDATER_COMPARE_ERROR)) {
-      LOG_WARN(("Could not write update.status updater compare failure."));
-    }
-    return FALSE;
-  }
-
-  // Check to make sure the updater.exe module has the unique updater identity.
-  // This is a security measure to make sure that the signed executable that
-  // we will run is actually an updater.
-  HMODULE updaterModule = LoadLibraryEx(argv[0], nullptr,
-                                        LOAD_LIBRARY_AS_DATAFILE);
-  if (!updaterModule) {
-    LOG_WARN(("updater.exe module could not be loaded. (%d)", GetLastError()));
-    result = FALSE;
-  } else {
-    char updaterIdentity[64];
-    if (!LoadStringA(updaterModule, IDS_UPDATER_IDENTITY,
-                     updaterIdentity, sizeof(updaterIdentity))) {
-      LOG_WARN(("The updater.exe application does not contain the Mozilla"
-                " updater identity."));
-      result = FALSE;
-    }
-
-    if (strcmp(updaterIdentity, UPDATER_IDENTITY_STRING)) {
-      LOG_WARN(("The updater.exe identity string is not valid."));
-      result = FALSE;
-    }
-    FreeLibrary(updaterModule);
-  }
-
-  if (result) {
-    LOG(("The updater.exe application contains the Mozilla"
-          " updater identity."));
-  } else {
-    if (!WriteStatusFailure(argv[1],
-                            SERVICE_UPDATER_IDENTITY_ERROR)) {
-      LOG_WARN(("Could not write update.status no updater identity."));
-    }
-    return TRUE;
-  }
-
-  // Check for updater.exe sign problems
-  BOOL updaterSignProblem = FALSE;
-#ifndef DISABLE_UPDATER_AUTHENTICODE_CHECK
-  updaterSignProblem = !DoesBinaryMatchAllowedCertificates(installDir,
-                                                           argv[0]);
-#endif
-
-  // Only proceed with the update if we have no signing problems
-  if (!updaterSignProblem) {
+  if (UpdaterIsValid(argv[0], installDir, argv[1])) {
     BOOL updateProcessWasStarted = FALSE;
     if (StartUpdateProcess(argc, argv, installDir,
                            updateProcessWasStarted)) {
       LOG(("updater.exe was launched and run successfully!"));
       LogFlush();
 
       // Don't attempt to update the service when the update is being staged.
       if (!IsUpdateBeingStaged(argc, argv)) {
@@ -579,55 +584,74 @@ ExecuteServiceCommand(int argc, LPWSTR *
     UuidToString(&guid, &guidString);
   }
   LOG(("Executing service command %ls, ID: %ls",
        argv[2], reinterpret_cast<LPCWSTR>(guidString)));
   RpcStringFree(&guidString);
 
   BOOL result = FALSE;
   if (!lstrcmpi(argv[2], L"software-update")) {
-
-    // Use the passed in command line arguments for the update, except for the
-    // path to updater.exe.  We copy updater.exe to a the directory of the
-    // MozillaMaintenance service so that a low integrity process cannot
+    // Use the passed in command line arguments for the update, except for the
+    // path to updater.exe. We always look for updater.exe in the installation
+    // directory, then we copy updater.exe to a the directory of the
+    // MozillaMaintenance service so that a low integrity process cannot
     // replace the updater.exe at any point and use that for the update.
     // It also makes DLL injection attacks harder.
-    LPWSTR oldUpdaterPath = argv[3];
+    WCHAR installDir[MAX_PATH + 1] = { L'\0' };
+    if (!GetInstallationDir(argc - 3, argv + 3, installDir)) {
+      LOG_WARN(("Could not get the installation directory"));
+      if (!WriteStatusFailure(argv[1],
+        SERVICE_INSTALLDIR_ERROR)) {
+        LOG_WARN(("Could not write update.status for GetInstallationDir failure."));
+      }
+      return FALSE;
+    }
+    WCHAR installDirUpdater[MAX_PATH + 1] = { L'\0' };
+    wcsncpy(installDirUpdater, installDir, MAX_PATH);
+    if (!PathAppendSafe(installDirUpdater, L"updater.exe")) {
+      LOG_WARN(("Install directory updater could not be determined."));
+      result = FALSE;
+    }
+
+    result = UpdaterIsValid(installDirUpdater, installDir, argv[5]);
+
     WCHAR secureUpdaterPath[MAX_PATH + 1] = { L'\0' };
-    result = GetSecureUpdaterPath(secureUpdaterPath); // Does its own logging
+    if (result) {
+      result = GetSecureUpdaterPath(secureUpdaterPath); // Does its own logging
+    }
     if (result) {
       LOG(("Passed in path: '%ls'; Using this path for updating: '%ls'.",
-           oldUpdaterPath, secureUpdaterPath));
+        installDirUpdater, secureUpdaterPath));
       DeleteSecureUpdater(secureUpdaterPath);
-      result = CopyFileW(oldUpdaterPath, secureUpdaterPath, FALSE);
+      result = CopyFileW(installDirUpdater, secureUpdaterPath, FALSE);
     }
 
     if (!result) {
       LOG_WARN(("Could not copy path to secure location.  (%d)",
                 GetLastError()));
       if (argc > 4 && !WriteStatusFailure(argv[4],
                                           SERVICE_COULD_NOT_COPY_UPDATER)) {
         LOG_WARN(("Could not write update.status could not copy updater error"));
       }
     } else {
 
       // We obtained the path and copied it successfully, update the path to
       // use for the service update.
       argv[3] = secureUpdaterPath;
 
-      WCHAR oldUpdaterINIPath[MAX_PATH + 1] = { L'\0' };
+      WCHAR installDirUpdaterINIPath[MAX_PATH + 1] = { L'\0' };
       WCHAR secureUpdaterINIPath[MAX_PATH + 1] = { L'\0' };
       if (PathGetSiblingFilePath(secureUpdaterINIPath, secureUpdaterPath,
                                  L"updater.ini") &&
-          PathGetSiblingFilePath(oldUpdaterINIPath, oldUpdaterPath,
+          PathGetSiblingFilePath(installDirUpdaterINIPath, installDirUpdater,
                                  L"updater.ini")) {
         // This is non fatal if it fails there is no real harm
-        if (!CopyFileW(oldUpdaterINIPath, secureUpdaterINIPath, FALSE)) {
+        if (!CopyFileW(installDirUpdaterINIPath, secureUpdaterINIPath, FALSE)) {
           LOG_WARN(("Could not copy updater.ini from: '%ls' to '%ls'.  (%d)",
-                    oldUpdaterINIPath, secureUpdaterINIPath, GetLastError()));
+                    installDirUpdaterINIPath, secureUpdaterINIPath, GetLastError()));
         }
       }
 
       result = ProcessSoftwareUpdateCommand(argc - 3, argv + 3);
       DeleteSecureUpdater(secureUpdaterPath);
     }
 
     // We might not reach here if the service install succeeded