Bug 945192. r=rstrong

toolkit/components/maintenanceservice/workmonitor.cpp

@@ -30,8 +30,6 @@
 static const int TIME_TO_WAIT_ON_UPDATER = 15 * 60 * 1000;
 char16_t* MakeCommandLine(int argc, char16_t **argv);
 BOOL WriteStatusFailure(LPCWSTR updateDirPath, int errorCode);
-BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer,  LPCWSTR siblingFilePath, 
-                            LPCWSTR newFileName);
 
 /* 
  * Read the update.status file and sets isApplying to true if

toolkit/components/telemetry/Histograms.json

@@ -3321,11 +3321,17 @@
     "description": "Updater: The interval in days between the previous and the current background update check when the check was timer initiated"
   },
   "UPDATER_STATUS_CODES": {
-    "expires_in_version": "never",
+    "expires_in_version": "35",
     "kind": "enumerated",
     "n_values": 50,
     "description": "Updater: the status of the latest update performed"
   },
+  "UPDATER_ALL_STATUS_CODES": {
+    "expires_in_version": "never",
+    "kind": "enumerated",
+    "n_values": 200,
+    "description": "Updater: the status of the latest update performed"
+  },
   "UPDATER_UPDATES_ENABLED": {
     "expires_in_version": "never",
     "kind": "boolean",

toolkit/mozapps/update/common/errors.h

@@ -69,6 +69,11 @@
 #define WRITE_ERROR_SHARING_VIOLATION_NOPROCESSFORPID 47
 #define WRITE_ERROR_SHARING_VIOLATION_NOPID 48
 
+#define FOTA_FILE_OPERATION_ERROR 49
+#define FOTA_RECOVERY_ERROR 50
+
+#define SECURE_LOCATION_UPDATE_ERROR 51
+
 // The following error codes are only used by updater.exe
 // when a fallback key exists and XPCShell tests are being run.
 #define FALLBACKKEY_UNKNOWN_ERROR 100

toolkit/mozapps/update/common/uachelper.cpp

@@ -4,6 +4,7 @@
 
 #include <windows.h>
 #include <wtsapi32.h>
+#include <aclapi.h>
 #include "uachelper.h"
 #include "updatelogging.h"
 
@@ -220,3 +221,93 @@ UACHelper::CanUserElevate()
 
   return canElevate;
 }
+
+/**
+ * Denies write access for everyone on the specified path.
+ *
+ * @param path The file path to modify the DACL on
+ * @param originalACL out parameter, set only if successful.
+ *                    caller must free.
+ * @return true on success
+ */
+bool
+UACHelper::DenyWriteACLOnPath(LPCWSTR path, PACL *originalACL,
+                              PSECURITY_DESCRIPTOR *sd)
+{
+  // Get the old security information on the path.
+  // Note that the actual buffer to be freed is contained in *sd.
+  // originalACL points within *sd's buffer.
+  *originalACL = nullptr;
+  *sd = nullptr;
+  DWORD result =
+    GetNamedSecurityInfoW(path, SE_FILE_OBJECT, DACL_SECURITY_INFORMATION,
+                          nullptr, nullptr, originalACL, nullptr, sd);
+  if (result != ERROR_SUCCESS) {
+    *sd = nullptr;
+    *originalACL = nullptr;
+    return false;
+  }
+
+  // Adjust the security for everyone to deny write
+  EXPLICIT_ACCESSW ea;
+  ZeroMemory(&ea, sizeof(EXPLICIT_ACCESSW));
+  ea.grfAccessPermissions = FILE_APPEND_DATA | FILE_WRITE_ATTRIBUTES |
+                            FILE_WRITE_DATA | FILE_WRITE_EA;
+  ea.grfAccessMode = DENY_ACCESS;
+  ea.grfInheritance = NO_INHERITANCE;
+  ea.Trustee.TrusteeForm = TRUSTEE_IS_NAME;
+  ea.Trustee.TrusteeType = TRUSTEE_IS_GROUP;
+  ea.Trustee.ptstrName = L"EVERYONE";
+  PACL dacl = nullptr;
+  result = SetEntriesInAclW(1, &ea, *originalACL, &dacl);
+  if (result != ERROR_SUCCESS) {
+    LocalFree(*sd);
+    *originalACL = nullptr;
+    *sd = nullptr;
+    return false;
+  }
+
+  // Update the path to have a the new DACL
+  result = SetNamedSecurityInfoW(const_cast<LPWSTR>(path), SE_FILE_OBJECT,
+                                 DACL_SECURITY_INFORMATION, nullptr, nullptr,
+                                 dacl, nullptr);
+  LocalFree(dacl);
+  return result == ERROR_SUCCESS;
+}
+
+/**
+ * Determines if the specified directory only has updater.exe inside of it,
+ * and nothing else.
+ *
+ * @param inputPath the directory path to search
+ * @return true if updater.exe is the only file in the directory
+ */
+bool
+UACHelper::IsDirectorySafe(LPCWSTR inputPath)
+{
+  WIN32_FIND_DATAW findData;
+  HANDLE findHandle = nullptr;
+
+  WCHAR searchPath[MAX_PATH + 1] = { L'\0' };
+  wsprintfW(searchPath, L"%s\\*.*", inputPath);
+
+  findHandle = FindFirstFileW(searchPath, &findData);
+  if(findHandle == INVALID_HANDLE_VALUE) {
+    return false;
+  }
+
+  // Enumerate the files and if we find anything other than the current
+  // directory, the parent directory, or updater.exe. Then fail.
+  do {
+    if(wcscmp(findData.cFileName, L".") != 0 &&
+       wcscmp(findData.cFileName, L"..") != 0 &&
+       wcscmp(findData.cFileName, L"updater.exe") != 0) {
+         FindClose(findHandle);
+      return false;
+    }
+  } while(FindNextFileW(findHandle, &findData));
+  FindClose(findHandle);
+
+  return true;
+}
+

toolkit/mozapps/update/common/uachelper.h

@@ -12,12 +12,15 @@ public:
   static HANDLE OpenLinkedToken(HANDLE token);
   static BOOL DisablePrivileges(HANDLE token);
   static bool CanUserElevate();
+  static bool IsDirectorySafe(LPCWSTR inputPath);
+  static bool DenyWriteACLOnPath(LPCWSTR path, PACL *originalACL,
+                                 PSECURITY_DESCRIPTOR *sd);
 
 private:
   static BOOL SetPrivilege(HANDLE token, LPCTSTR privs, BOOL enable);
-  static BOOL DisableUnneededPrivileges(HANDLE token, 
+  static BOOL DisableUnneededPrivileges(HANDLE token,
                                         LPCTSTR *unneededPrivs, size_t count);
-  static LPCTSTR PrivsToDisable[];  
+  static LPCTSTR PrivsToDisable[];
 };
 
 #endif

toolkit/mozapps/update/common/updatehelper.cpp

@@ -33,7 +33,6 @@ using mozilla::MakeUnique;
 using mozilla::UniquePtr;
 
 WCHAR* MakeCommandLine(int argc, WCHAR **argv);
-BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
 
 /**
  * Obtains the path of a file in the same directory as the specified file.

toolkit/mozapps/update/common/updatehelper.h

@@ -17,6 +17,10 @@ BOOL DoesFallbackKeyExist();
 BOOL IsLocalFile(LPCWSTR file, BOOL &isLocal);
 DWORD StartServiceCommand(int argc, LPCWSTR* argv);
 BOOL IsUnpromptedElevation(BOOL &isUnpromptedElevation);
+BOOL PathGetSiblingFilePath(LPWSTR destinationBuffer,
+                            LPCWSTR siblingFilePath,
+                            LPCWSTR newFileName);
+BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
 
 #define SVC_NAME L"MozillaMaintenance"
 

toolkit/mozapps/update/nsUpdateService.js

@@ -162,6 +162,7 @@ const WRITE_ERROR_SHARING_VIOLATION_NOPROCESSFORPID = 47;
 const WRITE_ERROR_SHARING_VIOLATION_NOPID           = 48;
 const FOTA_FILE_OPERATION_ERROR                     = 49;
 const FOTA_RECOVERY_ERROR                           = 50;
+const SECURE_LOCATION_UPDATE_ERROR                  = 51;
 
 const CERT_ATTR_CHECK_FAILED_NO_UPDATE  = 100;
 const CERT_ATTR_CHECK_FAILED_HAS_UPDATE = 101;
@@ -1477,7 +1478,8 @@ function handleUpdateFailure(update, errorCode) {
     return true;
   }
 
-  if (update.errorCode == ELEVATION_CANCELED) {
+  if (update.errorCode == ELEVATION_CANCELED ||
+      update.errorCode == SECURE_LOCATION_UPDATE_ERROR) {
     writeStatusFile(getUpdatesDir(), update.state = STATE_PENDING);
     return true;
   }
@@ -2383,7 +2385,7 @@ UpdateService.prototype = {
       if (parts.length > 1) {
         result = parseInt(parts[1]) || INVALID_UPDATER_STATUS_CODE;
       }
-      Services.telemetry.getHistogramById("UPDATER_STATUS_CODES").add(result);
+      Services.telemetry.getHistogramById("UPDATER_ALL_STATUS_CODES").add(result);
     } catch(e) {
       // Don't allow any exception to be propagated.
       Cu.reportError(e);

toolkit/mozapps/update/updater/moz.build

@@ -41,6 +41,8 @@ if CONFIG['OS_ARCH'] == 'WINNT':
         'shlwapi',
         'crypt32',
         'advapi32',
+        'ole32',
+        'rpcrt4',
     ]
 else:
     USE_LIBS += [

toolkit/mozapps/update/updater/updater.cpp

@@ -111,6 +111,7 @@ static bool sUseHardLinks = true;
 
 #ifdef XP_WIN
 #include "updatehelper.h"
+#include <aclapi.h>
 
 // Closes the handle if valid and if the updater is elevated returns with the
 // return code specified. This prevents multiple launches of the callback
@@ -2801,35 +2802,141 @@ int NS_main(int argc, NS_tchar **argv)
         }
       }
 
-      // If we didn't want to use the service at all, or if an update was 
-      // already happening, or launching the service command failed, then 
+      // If we didn't want to use the service at all, or if an update was
+      // already happening, or launching the service command failed, then
       // launch the elevated updater.exe as we do without the service.
       // We don't launch the elevated updater in the case that we did have
       // write access all along because in that case the only reason we're
-      // using the service is because we are testing. 
-      if (!useService && !noServiceFallback && 
+      // using the service is because we are testing.
+      if (!useService && !noServiceFallback &&
           updateLockFileHandle == INVALID_HANDLE_VALUE) {
-        SHELLEXECUTEINFO sinfo;
-        memset(&sinfo, 0, sizeof(SHELLEXECUTEINFO));
-        sinfo.cbSize       = sizeof(SHELLEXECUTEINFO);
-        sinfo.fMask        = SEE_MASK_FLAG_NO_UI |
-                             SEE_MASK_FLAG_DDEWAIT |
-                             SEE_MASK_NOCLOSEPROCESS;
-        sinfo.hwnd         = nullptr;
-        sinfo.lpFile       = argv[0];
-        sinfo.lpParameters = cmdLine;
-        sinfo.lpVerb       = L"runas";
-        sinfo.nShow        = SW_SHOWNORMAL;
 
-        bool result = ShellExecuteEx(&sinfo);
-        free(cmdLine);
-
-        if (result) {
-          WaitForSingleObject(sinfo.hProcess, INFINITE);
-          CloseHandle(sinfo.hProcess);
+        // Get a unique directory name to secure
+        RPC_WSTR guidString = RPC_WSTR(L"");
+        GUID guid;
+        HRESULT hr = CoCreateGuid(&guid);
+        BOOL result = TRUE;
+        bool safeToUpdate = true;
+        WCHAR secureUpdaterPath[MAX_PATH + 1] = { L'\0' };
+        WCHAR secureDirPath[MAX_PATH + 1] = { L'\0' };
+        if (SUCCEEDED(hr)) {
+          UuidToString(&guid, &guidString);
+          result = PathGetSiblingFilePath(secureDirPath, argv[0],
+                                          reinterpret_cast<LPCWSTR>(guidString));
+          RpcStringFree(&guidString);
         } else {
-          WriteStatusFile(ELEVATION_CANCELED);
+          // This should never happen, but just in case
+          result = PathGetSiblingFilePath(secureDirPath, argv[0], L"tmp_update");
         }
+
+        if (!result) {
+          fprintf(stderr, "Could not obtain secure update directory path");
+          safeToUpdate = false;
+        }
+
+        // If it's still safe to update, create the directory
+        if (safeToUpdate) {
+          result = CreateDirectoryW(secureDirPath, nullptr);
+          if (!result) {
+            fprintf(stderr, "Could not create secure update directory");
+            safeToUpdate = false;
+          }
+        }
+
+        // If it's still safe to update, get the new updater path
+        if (safeToUpdate) {
+          wcsncpy(secureUpdaterPath, secureDirPath, MAX_PATH);
+          result = PathAppendSafe(secureUpdaterPath, L"updater.exe");
+          if (!result) {
+            fprintf(stderr, "Could not obtain secure updater file name");
+            safeToUpdate = false;
+          }
+        }
+
+        // If it's still safe to update, copy the file in
+        if (safeToUpdate) {
+          result = CopyFileW(argv[0], secureUpdaterPath, TRUE);
+          if (!result) {
+            fprintf(stderr, "Could not copy updater to secure location");
+            safeToUpdate = false;
+          }
+        }
+
+        // If it's still safe to update, restrict access to the directory item
+        // itself so that the directory cannot be deleted and re-created,
+        // nor have its properties modified.  Note that this does not disallow
+        // adding items inside the directory.
+        HANDLE handle = INVALID_HANDLE_VALUE;
+        if (safeToUpdate) {
+          handle = CreateFileW(secureDirPath, GENERIC_READ, FILE_SHARE_READ,
+                               nullptr, OPEN_EXISTING,
+                               FILE_FLAG_BACKUP_SEMANTICS, nullptr);
+          safeToUpdate = handle != INVALID_HANDLE_VALUE;
+        }
+
+        // If it's still safe to update, deny write access completely to the
+        // directory.
+        PACL originalACL = nullptr;
+        PSECURITY_DESCRIPTOR sd = nullptr;
+        if (safeToUpdate) {
+          safeToUpdate = UACHelper::DenyWriteACLOnPath(secureDirPath,
+                                                       &originalACL, &sd);
+        }
+
+        // If it's still safe to update, verify that there is only updater.exe
+        // in the directory and nothing else.
+        if (safeToUpdate) {
+          if (!UACHelper::IsDirectorySafe(secureDirPath)) {
+            safeToUpdate = false;
+          }
+        }
+
+        if (!safeToUpdate) {
+          fprintf(stderr, "Will not proceed to copy secure updater because it "
+                          "is not safe to do so.");
+          WriteStatusFile(SECURE_LOCATION_UPDATE_ERROR);
+        } else {
+          SHELLEXECUTEINFO sinfo;
+          memset(&sinfo, 0, sizeof(SHELLEXECUTEINFO));
+          sinfo.cbSize       = sizeof(SHELLEXECUTEINFO);
+          sinfo.fMask        = SEE_MASK_FLAG_NO_UI |
+                              SEE_MASK_FLAG_DDEWAIT |
+                              SEE_MASK_NOCLOSEPROCESS;
+          sinfo.hwnd         = nullptr;
+          sinfo.lpFile       = secureUpdaterPath;
+          sinfo.lpParameters = cmdLine;
+          sinfo.lpVerb       = L"runas";
+          sinfo.nShow        = SW_SHOWNORMAL;
+
+          bool result = ShellExecuteEx(&sinfo);
+          free(cmdLine);
+
+          if (result) {
+            WaitForSingleObject(sinfo.hProcess, INFINITE);
+            CloseHandle(sinfo.hProcess);
+          } else {
+            WriteStatusFile(ELEVATION_CANCELED);
+          }
+        }
+
+        // All done, revert back the permissions.
+        if (originalACL) {
+          SetNamedSecurityInfoW(const_cast<LPWSTR>(secureDirPath), SE_FILE_OBJECT,
+                                DACL_SECURITY_INFORMATION, nullptr, nullptr,
+                                originalACL, nullptr);
+        }
+        if (sd) {
+          LocalFree(sd);
+        }
+
+        // Done with the directory, no need to lock it.
+        if (INVALID_HANDLE_VALUE != handle) {
+          CloseHandle(handle);
+        }
+
+        // We no longer need the secure updater and directory
+        DeleteFileW(secureUpdaterPath);
+        RemoveDirectoryW(secureDirPath);
       }
 
       if (argc > callbackIndex) {