Client code - Bug 1342742 - check that the app update patch dir, install dir, and working dir paths are valid. r=mhowell, a=rkothari
authorRobert Strong <robert.bugzilla@gmail.com>
Wed, 03 May 2017 16:31:16 -0700
changeset 393801 f7d1810de381
parent 393800 b29bc19a99d5
child 393802 bbcddab3288a
push id7262
push userrstrong@mozilla.com
push dateWed, 03 May 2017 23:33:19 +0000
treeherdermozilla-beta@4969ec22e2b5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmhowell, rkothari
bugs1342742
milestone54.0
Client code - Bug 1342742 - check that the app update patch dir, install dir, and working dir paths are valid. r=mhowell, a=rkothari
toolkit/components/maintenanceservice/servicebase.h
toolkit/components/maintenanceservice/workmonitor.cpp
toolkit/mozapps/update/common/certificatecheck.cpp
toolkit/mozapps/update/common/errors.h
toolkit/mozapps/update/common/moz.build
toolkit/mozapps/update/common/registrycertificates.cpp
toolkit/mozapps/update/common/sources.mozbuild
toolkit/mozapps/update/common/uachelper.cpp
toolkit/mozapps/update/common/updatecommon.cpp
toolkit/mozapps/update/common/updatecommon.h
toolkit/mozapps/update/common/updatehelper.cpp
toolkit/mozapps/update/common/updatelogging.cpp
toolkit/mozapps/update/common/updatelogging.h
toolkit/mozapps/update/updater/launchchild_osx.mm
toolkit/mozapps/update/updater/updater.cpp
--- a/toolkit/components/maintenanceservice/servicebase.h
+++ b/toolkit/components/maintenanceservice/servicebase.h
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <windows.h>
-#include "updatelogging.h"
+#include "updatecommon.h"
 
 BOOL PathAppendSafe(LPWSTR base, LPCWSTR extra);
 BOOL VerifySameFiles(LPCWSTR file1Path, LPCWSTR file2Path, BOOL &sameContent);
 
 // 32KiB for comparing files at a time seems reasonable.
 // The bigger the better for speed, but this will be used
 // on the stack so I don't want it to be too big.
 #define COMPARE_BLOCKSIZE 32768
--- a/toolkit/components/maintenanceservice/workmonitor.cpp
+++ b/toolkit/components/maintenanceservice/workmonitor.cpp
@@ -17,26 +17,28 @@
 #include "nsWindowsHelpers.h"
 
 #include "workmonitor.h"
 #include "serviceinstall.h"
 #include "servicebase.h"
 #include "registrycertificates.h"
 #include "uachelper.h"
 #include "updatehelper.h"
+#include "pathhash.h"
 #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,
                             LPCWSTR newFileName);
+BOOL DoesFallbackKeyExist();
 
 /*
  * 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
  *         is set to applying or not.
@@ -410,29 +412,27 @@ ProcessSoftwareUpdateCommand(DWORD argc,
   BOOL result = TRUE;
   if (argc < 3) {
     LOG_WARN(("Not enough command line parameters specified. "
               "Updating update.status."));
 
     // We can only update update.status if argv[1] exists.  argv[1] is
     // the directory where the update.status file exists.
     if (argc < 2 ||
-        !WriteStatusFailure(argv[1],
-                            SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS)) {
+        !WriteStatusFailure(argv[1], SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS)) {
       LOG_WARN(("Could not write update.status service update failure.  (%d)",
                 GetLastError()));
     }
     return FALSE;
   }
 
   WCHAR installDir[MAX_PATH + 1] = {L'\0'};
   if (!GetInstallationDir(argc, argv, installDir)) {
     LOG_WARN(("Could not get the installation directory"));
-    if (!WriteStatusFailure(argv[1],
-                            SERVICE_INSTALLDIR_ERROR)) {
+    if (!WriteStatusFailure(argv[1], SERVICE_INSTALLDIR_ERROR)) {
       LOG_WARN(("Could not write update.status for GetInstallationDir failure."));
     }
     return FALSE;
   }
 
   if (UpdaterIsValid(argv[0], installDir, argv[1])) {
     BOOL updateProcessWasStarted = FALSE;
     if (StartUpdateProcess(argc, argv, installDir,
@@ -582,56 +582,147 @@ 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")) {
+    // This check is also performed in updater.cpp and is performed here
+    // as well since the maintenance service can be called directly.
+    if (argc < 4 || !IsValidFullPath(argv[4])) {
+      // Since the status file is written to the patch directory and the patch
+      // directory is invalid don't write the status file.
+      LOG_WARN(("The patch directory path is not valid for this application."));
+      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])) {
+      LOG_WARN(("The install directory path is not valid for this application."));
+      if (!WriteStatusFailure(argv[4], SERVICE_INVALID_INSTALL_DIR_PATH_ERROR)) {
+        LOG_WARN(("Could not write update.status for previous failure."));
+      }
+      return FALSE;
+    }
+
+    if (!IsOldCommandline(argc - 3, argv + 3)) {
+      // This check is also performed in updater.cpp and is performed here
+      // as well since the maintenance service can be called directly.
+      if (argc < 6 || !IsValidFullPath(argv[6])) {
+        LOG_WARN(("The working directory path is not valid for this application."));
+        if (!WriteStatusFailure(argv[4], SERVICE_INVALID_WORKING_DIR_PATH_ERROR)) {
+          LOG_WARN(("Could not write update.status for previous failure."));
+        }
+        return FALSE;
+      }
+
+      // These checks are also performed in updater.cpp and is performed here
+      // as well since the maintenance service can be called directly.
+      if (_wcsnicmp(argv[6], argv[5], MAX_PATH) != 0) {
+        if (wcscmp(argv[7], L"-1") != 0 && !wcsstr(argv[7], L"/replace")) {
+          LOG_WARN(("Installation directory and working directory must be the "
+                    "same for non-staged updates. Exiting."));
+          if (!WriteStatusFailure(argv[4], SERVICE_INVALID_APPLYTO_DIR_ERROR)) {
+            LOG_WARN(("Could not write update.status for previous failure."));
+          }
+          return FALSE;
+        }
+
+        NS_tchar workingDirParent[MAX_PATH];
+        NS_tsnprintf(workingDirParent,
+                     sizeof(workingDirParent) / sizeof(workingDirParent[0]),
+                     NS_T("%s"), argv[6]);
+        if (!PathRemoveFileSpecW(workingDirParent)) {
+          LOG_WARN(("Couldn't remove file spec when attempting to verify the "
+                    "working directory path.  (%d)", GetLastError()));
+          if (!WriteStatusFailure(argv[4], REMOVE_FILE_SPEC_ERROR)) {
+            LOG_WARN(("Could not write update.status for previous failure."));
+          }
+          return FALSE;
+        }
+
+        if (_wcsnicmp(workingDirParent, argv[5], MAX_PATH) != 0) {
+          LOG_WARN(("The apply-to directory must be the same as or "
+                    "a child of the installation directory! Exiting."));
+          if (!WriteStatusFailure(argv[4], SERVICE_INVALID_APPLYTO_DIR_STAGED_ERROR)) {
+            LOG_WARN(("Could not write update.status for previous failure."));
+          }
+          return FALSE;
+        }
+      }
+
+    }
+
     // 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.
     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."));
+      if (!WriteStatusFailure(argv[4], SERVICE_INSTALLDIR_ERROR)) {
+        LOG_WARN(("Could not write update.status for previous failure."));
       }
       return FALSE;
     }
+
+    if (!DoesFallbackKeyExist()) {
+      WCHAR maintenanceServiceKey[MAX_PATH + 1];
+      if (CalculateRegistryPathFromFilePath(installDir, maintenanceServiceKey)) {
+        LOG(("Checking for Maintenance Service registry. key: '%ls'",
+             maintenanceServiceKey));
+        HKEY baseKey = nullptr;
+        if (RegOpenKeyExW(HKEY_LOCAL_MACHINE,
+                          maintenanceServiceKey, 0,
+                          KEY_READ | KEY_WOW64_64KEY,
+                          &baseKey) != ERROR_SUCCESS) {
+          LOG_WARN(("The maintenance service registry key does not exist."));
+          if (!WriteStatusFailure(argv[4], SERVICE_INSTALL_DIR_REG_ERROR)) {
+            LOG_WARN(("Could not write update.status for previous failure."));
+          }
+          return FALSE;
+        }
+        RegCloseKey(baseKey);
+      } else {
+        if (!WriteStatusFailure(argv[4], SERVICE_CALC_REG_PATH_ERROR)) {
+          LOG_WARN(("Could not write update.status for previous 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]);
+    result = UpdaterIsValid(installDirUpdater, installDir, argv[4]);
 
     WCHAR secureUpdaterPath[MAX_PATH + 1] = { L'\0' };
     if (result) {
       result = GetSecureUpdaterPath(secureUpdaterPath); // Does its own logging
     }
     if (result) {
       LOG(("Passed in path: '%ls'; Using this path for updating: '%ls'.",
-        installDirUpdater, secureUpdaterPath));
+           installDirUpdater, secureUpdaterPath));
       DeleteSecureUpdater(secureUpdaterPath);
       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)) {
+      if (!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;
 
--- a/toolkit/mozapps/update/common/certificatecheck.cpp
+++ b/toolkit/mozapps/update/common/certificatecheck.cpp
@@ -4,17 +4,17 @@
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <windows.h>
 #include <softpub.h>
 #include <wintrust.h>
 
 #include "certificatecheck.h"
-#include "updatelogging.h"
+#include "updatecommon.h"
 
 static const int ENCODING = X509_ASN_ENCODING | PKCS_7_ASN_ENCODING;
 
 /**
  * Checks to see if a file stored at filePath matches the specified info.
  *
  * @param  filePath    The PE file path to check
  * @param  infoToMatch The acceptable information to match
--- a/toolkit/mozapps/update/common/errors.h
+++ b/toolkit/mozapps/update/common/errors.h
@@ -36,17 +36,17 @@
 #define CERT_LOAD_ERROR 17
 #define CERT_HANDLING_ERROR 18
 #define CERT_VERIFY_ERROR 19
 #define ARCHIVE_NOT_OPEN 20
 #define COULD_NOT_READ_PRODUCT_INFO_BLOCK_ERROR 21
 #define MAR_CHANNEL_MISMATCH_ERROR 22
 #define VERSION_DOWNGRADE_ERROR 23
 
-// Error codes 24-33 and 49-51 are for the Windows maintenance service.
+// Error codes 24-33 and 49-57 are for the Windows maintenance service.
 #define SERVICE_UPDATER_COULD_NOT_BE_STARTED 24
 #define SERVICE_NOT_ENOUGH_COMMAND_LINE_ARGS 25
 #define SERVICE_UPDATER_SIGN_ERROR 26
 #define SERVICE_UPDATER_COMPARE_ERROR 27
 #define SERVICE_UPDATER_IDENTITY_ERROR 28
 #define SERVICE_STILL_APPLYING_ON_SUCCESS 29
 #define SERVICE_STILL_APPLYING_ON_FAILURE 30
 #define SERVICE_UPDATER_NOT_FIXED_DRIVE 31
@@ -61,35 +61,43 @@
 #define UNEXPECTED_MAR_ERROR 40
 #define UNEXPECTED_BSPATCH_ERROR 41
 #define UNEXPECTED_FILE_OPERATION_ERROR 42
 #define FILESYSTEM_MOUNT_READWRITE_ERROR 43
 #define DELETE_ERROR_EXPECTED_DIR 46
 #define DELETE_ERROR_EXPECTED_FILE 47
 #define RENAME_ERROR_EXPECTED_FILE 48
 
-// Error codes 24-33 and 49-51 are for the Windows maintenance service.
+// Error codes 24-33 and 49-57 are for the Windows maintenance service.
 #define SERVICE_COULD_NOT_COPY_UPDATER 49
 #define SERVICE_STILL_APPLYING_TERMINATED 50
 #define SERVICE_STILL_APPLYING_NO_EXIT_CODE 51
+#define SERVICE_INVALID_APPLYTO_DIR_STAGED_ERROR 52
+#define SERVICE_CALC_REG_PATH_ERROR 53
+#define SERVICE_INVALID_APPLYTO_DIR_ERROR 54
+#define SERVICE_INVALID_INSTALL_DIR_PATH_ERROR 55
+#define SERVICE_INVALID_WORKING_DIR_PATH_ERROR 56
+#define SERVICE_INSTALL_DIR_REG_ERROR 57
 
 #define WRITE_ERROR_FILE_COPY 61
 #define WRITE_ERROR_DELETE_FILE 62
 #define WRITE_ERROR_OPEN_PATCH_FILE 63
 #define WRITE_ERROR_PATCH_FILE 64
 #define WRITE_ERROR_APPLY_DIR_PATH 65
 #define WRITE_ERROR_CALLBACK_PATH 66
 #define WRITE_ERROR_FILE_ACCESS_DENIED 67
 #define WRITE_ERROR_DIR_ACCESS_DENIED 68
 #define WRITE_ERROR_DELETE_BACKUP 69
 #define WRITE_ERROR_EXTRACT 70
 #define REMOVE_FILE_SPEC_ERROR 71
 #define INVALID_APPLYTO_DIR_STAGED_ERROR 72
 #define LOCK_ERROR_PATCH_FILE 73
 #define INVALID_APPLYTO_DIR_ERROR 74
+#define INVALID_INSTALL_DIR_PATH_ERROR 75
+#define INVALID_WORKING_DIR_PATH_ERROR 76
 
 // Error codes 80 through 99 are reserved for nsUpdateService.js
 
 // The following error codes are only used by updater.exe
 // when a fallback key exists for tests.
 #define FALLBACKKEY_UNKNOWN_ERROR 100
 #define FALLBACKKEY_REGPATH_ERROR 101
 #define FALLBACKKEY_NOKEY_ERROR 102
--- a/toolkit/mozapps/update/common/moz.build
+++ b/toolkit/mozapps/update/common/moz.build
@@ -1,18 +1,18 @@
 # -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 EXPORTS += [
     'readstrings.h',
+    'updatecommon.h',
     'updatedefines.h',
-    'updatelogging.h',
 ]
 
 if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'windows':
     EXPORTS += [
         'pathhash.h',
         'uachelper.h',
         'updatehelper.cpp',
         'updatehelper.h',
--- a/toolkit/mozapps/update/common/registrycertificates.cpp
+++ b/toolkit/mozapps/update/common/registrycertificates.cpp
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <windows.h>
 
 #include "registrycertificates.h"
 #include "pathhash.h"
-#include "updatelogging.h"
+#include "updatecommon.h"
 #include "updatehelper.h"
 #define MAX_KEY_LENGTH 255
 
 /**
  * Verifies if the file path matches any certificate stored in the registry.
  *
  * @param  filePath The file path of the application to check if allowed.
  * @param  allowFallbackKeySkip when this is TRUE the fallback registry key will
--- a/toolkit/mozapps/update/common/sources.mozbuild
+++ b/toolkit/mozapps/update/common/sources.mozbuild
@@ -17,12 +17,12 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'wind
         ]
         OS_LIBS += [
             'crypt32',
             'wintrust',
         ]
 
 sources += [
     'readstrings.cpp',
-    'updatelogging.cpp',
+    'updatecommon.cpp',
 ]
 
 SOURCES += sorted(['%s/%s' % (srcdir, s) for s in sources])
--- a/toolkit/mozapps/update/common/uachelper.cpp
+++ b/toolkit/mozapps/update/common/uachelper.cpp
@@ -1,16 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include <windows.h>
 #include <wtsapi32.h>
 #include "uachelper.h"
-#include "updatelogging.h"
+#include "updatecommon.h"
 
 // See the MSDN documentation with title: Privilege Constants
 // At the time of this writing, this documentation is located at: 
 // http://msdn.microsoft.com/en-us/library/windows/desktop/bb530716%28v=vs.85%29.aspx
 LPCTSTR UACHelper::PrivsToDisable[] = { 
   SE_ASSIGNPRIMARYTOKEN_NAME,
   SE_AUDIT_NAME,
   SE_BACKUP_NAME,
rename from toolkit/mozapps/update/common/updatelogging.cpp
rename to toolkit/mozapps/update/common/updatecommon.cpp
--- a/toolkit/mozapps/update/common/updatelogging.cpp
+++ b/toolkit/mozapps/update/common/updatecommon.cpp
@@ -7,17 +7,17 @@
 #endif
 
 
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
 #include <stdarg.h>
 
-#include "updatelogging.h"
+#include "updatecommon.h"
 
 UpdateLog::UpdateLog() : logFP(nullptr)
 {
 }
 
 void UpdateLog::Init(NS_tchar* sourcePath,
                      const NS_tchar* fileName)
 {
@@ -29,22 +29,23 @@ void UpdateLog::Init(NS_tchar* sourcePat
     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
-    GetTempFileNameW(sourcePath, L"log", 0, mTmpFilePath);
-    logFP = NS_tfopen(mTmpFilePath, NS_T("w"));
+    if (GetTempFileNameW(sourcePath, L"log", 0, mTmpFilePath) != 0) {
+      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);
+      // 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
     logFP = NS_tfopen(mDstFilePath, NS_T("w"));
 #else
     // On platforms that have an updates directory in the installation directory
     // (e.g. platforms other than Windows and Mac) the update log is written to
     // a temporary file and then to the update log file. This is needed since
     // the installation directory is moved during a replace request. This can be
     // removed when the platform's updates directory is located outside of the
@@ -140,8 +141,73 @@ void UpdateLog::WarnPrintf(const char *f
 
   va_list ap;
   va_start(ap, fmt);
   fprintf(logFP, "*** Warning: ");
   vfprintf(logFP, fmt, ap);
   fprintf(logFP, "***\n");
   va_end(ap);
 }
+
+/**
+ * Performs checks of a full path for validity for this application.
+ *
+ * @param  origFullPath
+ *         The full path to check.
+ * @return true if the path is valid for this application and false otherwise.
+ */
+bool
+IsValidFullPath(NS_tchar* origFullPath)
+{
+  // Subtract 1 from MAXPATHLEN for null termination.
+  if (NS_tstrlen(origFullPath) > MAXPATHLEN - 1) {
+    // The path is longer than acceptable for this application.
+    return false;
+  }
+
+#ifdef XP_WIN
+  NS_tchar testPath[MAXPATHLEN] = {NS_T('\0')};
+  // GetFullPathNameW will replace / with \ which PathCanonicalizeW requires.
+  if (GetFullPathNameW(origFullPath, MAXPATHLEN, testPath, nullptr) == 0) {
+    // Unable to get the full name for the path (e.g. invalid path).
+    return false;
+  }
+
+  NS_tchar canonicalPath[MAXPATHLEN] = {NS_T('\0')};
+  if (!PathCanonicalizeW(canonicalPath, testPath)) {
+    // Path could not be canonicalized (e.g. invalid path).
+    return false;
+  }
+
+  // Check if the path passed in resolves to a differerent path.
+  if (NS_tstricmp(origFullPath, canonicalPath) != 0) {
+    // Case insensitive string comparison between the supplied path and the
+    // canonical path are not equal. This will prevent directory traversal and
+    // the use of / in paths since they are converted to \.
+    return false;
+  }
+
+  NS_tstrncpy(testPath, origFullPath, MAXPATHLEN);
+  if (!PathStripToRootW(testPath)) {
+    // It should always be possible to strip a valid path to its root.
+    return false;
+  }
+
+  if (origFullPath[0] == NS_T('\\')) {
+    // Only allow UNC server share paths.
+    if (!PathIsUNCServerShareW(testPath)) {
+      return false;
+    }
+  }
+#else
+  // Only allow full paths.
+  if (origFullPath[0] != NS_T('/')) {
+    return false;
+  }
+
+  // The path must not traverse directories
+  if (NS_tstrstr(origFullPath, NS_T("..")) != nullptr ||
+      NS_tstrstr(origFullPath, NS_T("./")) != nullptr) {
+    return false;
+  }
+#endif
+  return true;
+}
rename from toolkit/mozapps/update/common/updatelogging.h
rename to toolkit/mozapps/update/common/updatecommon.h
--- a/toolkit/mozapps/update/common/updatelogging.h
+++ b/toolkit/mozapps/update/common/updatecommon.h
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef UPDATELOGGING_H
-#define UPDATELOGGING_H
+#ifndef UPDATECOMMON_H
+#define UPDATECOMMON_H
 
 #include "updatedefines.h"
 #include <stdio.h>
 
 class UpdateLog
 {
 public:
   static UpdateLog & GetPrimaryLog() 
@@ -30,16 +30,18 @@ public:
 
 protected:
   UpdateLog();
   FILE *logFP;
   NS_tchar mTmpFilePath[MAXPATHLEN];
   NS_tchar mDstFilePath[MAXPATHLEN];
 };
 
+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 LogFinish() UpdateLog::GetPrimaryLog().Finish()
 #define LogFlush() UpdateLog::GetPrimaryLog().Flush()
 
 #endif
--- a/toolkit/mozapps/update/common/updatehelper.cpp
+++ b/toolkit/mozapps/update/common/updatehelper.cpp
@@ -268,17 +268,19 @@ PathAppendSafe(LPWSTR base, LPCWSTR extr
  * @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' };
-  GetTempFileNameW(updateDirPath, L"svc", 0, tmpUpdateStatusFilePath);
+  if (GetTempFileNameW(updateDirPath, L"svc", 0, tmpUpdateStatusFilePath) == 0) {
+    return FALSE;
+  }
 
   HANDLE tmpStatusFile = CreateFileW(tmpUpdateStatusFilePath, GENERIC_WRITE, 0,
                                      nullptr, CREATE_ALWAYS, 0, nullptr);
   if (tmpStatusFile == INVALID_HANDLE_VALUE) {
     return FALSE;
   }
 
   char failure[32];
--- a/toolkit/mozapps/update/updater/launchchild_osx.mm
+++ b/toolkit/mozapps/update/updater/launchchild_osx.mm
@@ -74,16 +74,23 @@ LaunchMacPostProcess(const char* aAppBun
   }
 
   NSString *exeRelPath = [NSString stringWithUTF8String:values[0]];
   NSString *exeArg = [NSString stringWithUTF8String:values[1]];
   if (!exeArg || !exeRelPath) {
     return;
   }
 
+  // The path must not traverse directories and it must be a relative path.
+  if ([exeRelPath rangeOfString:@".."].location != NSNotFound ||
+      [exeRelPath rangeOfString:@"./"].location != NSNotFound ||
+      [exeRelPath rangeOfString:@"/"].location == 0) {
+    return;
+  }
+
   NSString* exeFullPath = [NSString stringWithUTF8String:aAppBundle];
   exeFullPath = [exeFullPath stringByAppendingPathComponent:exeRelPath];
 
   char optVals[1][MAX_TEXT_LEN];
   readResult = ReadStrings([iniPath UTF8String],
                            "ExeAsync\0",
                            1,
                            optVals,
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -47,17 +47,17 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <errno.h>
 #include <algorithm>
 
-#include "updatelogging.h"
+#include "updatecommon.h"
 #ifdef XP_MACOSX
 #include "updaterfileutils_osx.h"
 #endif // XP_MACOSX
 
 #include "mozilla/Compiler.h"
 #include "mozilla/Types.h"
 #include "mozilla/UniquePtr.h"
 
@@ -301,17 +301,17 @@ private:
 };
 
 #else
 #error "Unsupported platform"
 #endif
 
 //-----------------------------------------------------------------------------
 
-static NS_tchar* gPatchDirPath;
+static NS_tchar gPatchDirPath[MAXPATHLEN];
 static NS_tchar gInstallDirPath[MAXPATHLEN];
 static NS_tchar gWorkingDirPath[MAXPATHLEN];
 static ArchiveReader gArchiveReader;
 static bool gSucceeded = false;
 static bool sStagedUpdate = false;
 static bool sReplaceRequest = false;
 static bool sUsingService = false;
 static bool sIsOSUpdate = false;
@@ -2014,27 +2014,43 @@ LaunchWinPostProcess(const WCHAR *instal
 
   if (!GetPrivateProfileStringW(L"PostUpdateWin", L"ExeAsync", L"TRUE",
                                 exeasync,
                                 sizeof(exeasync)/sizeof(exeasync[0]),
                                 inifile)) {
     return false;
   }
 
-  // Verify that exeFile doesn't contain relative paths
-  if (wcsstr(exefile, L"..") != nullptr) {
+  // The relative path must not contain directory traversals, current directory,
+  // or colons.
+  if (wcsstr(exefile, L"..") != nullptr ||
+      wcsstr(exefile, L"./") != nullptr ||
+      wcsstr(exefile, L".\\") != nullptr ||
+      wcsstr(exefile, L":") != nullptr) {
+    return false;
+  }
+
+  // The relative path must not start with a decimal point, backslash, or
+  // forward slash.
+  if (exefile[0] == L'.' ||
+      exefile[0] == L'\\' ||
+      exefile[0] == L'/') {
     return false;
   }
 
   WCHAR exefullpath[MAX_PATH + 1] = { L'\0' };
   wcsncpy(exefullpath, installationDir, MAX_PATH);
   if (!PathAppendSafe(exefullpath, exefile)) {
     return false;
   }
 
+  if (!IsValidFullPath(exefullpath)) {
+    return false;
+  }
+
 #if !defined(TEST_UPDATER) && defined(MOZ_MAINTENANCE_SERVICE)
   if (sUsingService &&
       !DoesBinaryMatchAllowedCertificates(installationDir, exefullpath)) {
     return false;
   }
 #endif
 
   WCHAR dlogFile[MAX_PATH + 1];
@@ -2131,17 +2147,19 @@ LaunchCallbackApp(const NS_tchar *workin
 
 static bool
 WriteStatusFile(const char* aStatus)
 {
   NS_tchar filename[MAXPATHLEN] = {NS_T('\0')};
 #if defined(XP_WIN)
   // The temp file is not removed on failure since there is client code that
   // will remove it.
-  GetTempFileNameW(gPatchDirPath, L"sta", 0, filename);
+  if (GetTempFileNameW(gPatchDirPath, L"sta", 0, filename) == 0) {
+    return false;
+  }
 #else
   NS_tsnprintf(filename, sizeof(filename)/sizeof(filename[0]),
                NS_T("%s/update.status"), gPatchDirPath);
 #endif
 
   // Make sure that the directory for the update status file exists
   if (ensure_parent_dir(filename)) {
     return false;
@@ -2817,19 +2835,48 @@ int NS_main(int argc, NS_tchar **argv)
     if (isElevated) {
       freeArguments(argc, argv);
       CleanupElevatedMacUpdate(true);
     }
 #endif
     return 1;
   }
 
+  // This check is also performed in workmonitor.cpp since the maintenance
+  // service can be called directly.
+  if (!IsValidFullPath(argv[1])) {
+    // Since the status file is written to the patch directory and the patch
+    // directory is invalid don't write the status file.
+    fprintf(stderr, "The patch directory path is not valid for this "  \
+            "application (" LOG_S ")\n", argv[1]);
+#ifdef XP_MACOSX
+    if (isElevated) {
+      freeArguments(argc, argv);
+      CleanupElevatedMacUpdate(true);
+    }
+#endif
+    return 1;
+  }
   // The directory containing the update information.
-  gPatchDirPath = argv[1];
-
+  NS_tstrncpy(gPatchDirPath, argv[1], MAXPATHLEN);
+
+  // This check is also performed in workmonitor.cpp since the maintenance
+  // service can be called directly.
+  if (!IsValidFullPath(argv[2])) {
+    WriteStatusFile(INVALID_INSTALL_DIR_PATH_ERROR);
+    fprintf(stderr, "The install directory path is not valid for this "  \
+            "application (" LOG_S ")\n", argv[2]);
+#ifdef XP_MACOSX
+    if (isElevated) {
+      freeArguments(argc, argv);
+      CleanupElevatedMacUpdate(true);
+    }
+#endif
+    return 1;
+  }
   // The directory we're going to update to.
   // We copy this string because we need to remove trailing slashes.  The C++
   // standard says that it's always safe to write to strings pointed to by argv
   // elements, but I don't necessarily believe it.
   NS_tstrncpy(gInstallDirPath, argv[2], MAXPATHLEN);
   gInstallDirPath[MAXPATHLEN - 1] = NS_T('\0');
   NS_tchar *slash = NS_tstrrchr(gInstallDirPath, NS_SLASH);
   if (slash && !slash[1]) {
@@ -2892,16 +2939,30 @@ int NS_main(int argc, NS_tchar **argv)
       sStagedUpdate = true;
     } else if (NS_tstrstr(argv[4], NS_T("/replace"))) {
       // We're processing a request to replace the application with a staged
       // update.
       sReplaceRequest = true;
     }
   }
 
+  // This check is also performed in workmonitor.cpp since the maintenance
+  // service can be called directly.
+  if (!IsValidFullPath(argv[3])) {
+    WriteStatusFile(INVALID_WORKING_DIR_PATH_ERROR);
+    fprintf(stderr, "The working directory path is not valid for this "  \
+            "application (" LOG_S ")\n", argv[3]);
+#ifdef XP_MACOSX
+    if (isElevated) {
+      freeArguments(argc, argv);
+      CleanupElevatedMacUpdate(true);
+    }
+#endif
+    return 1;
+  }
   // The directory we're going to update to.
   // We copy this string because we need to remove trailing slashes.  The C++
   // standard says that it's always safe to write to strings pointed to by argv
   // elements, but I don't necessarily believe it.
   NS_tstrncpy(gWorkingDirPath, argv[3], MAXPATHLEN);
   gWorkingDirPath[MAXPATHLEN - 1] = NS_T('\0');
   slash = NS_tstrrchr(gWorkingDirPath, NS_SLASH);
   if (slash && !slash[1]) {
@@ -2953,16 +3014,18 @@ int NS_main(int argc, NS_tchar **argv)
     LOG(("Performing a replace request"));
   }
 
   LOG(("PATCH DIRECTORY " LOG_S, gPatchDirPath));
   LOG(("INSTALLATION DIRECTORY " LOG_S, gInstallDirPath));
   LOG(("WORKING DIRECTORY " LOG_S, gWorkingDirPath));
 
 #if defined(XP_WIN)
+  // These checks are also performed in workmonitor.cpp since the maintenance
+  // service can be called directly.
   if (_wcsnicmp(gWorkingDirPath, gInstallDirPath, MAX_PATH) != 0) {
     if (!sStagedUpdate && !sReplaceRequest) {
       WriteStatusFile(INVALID_APPLYTO_DIR_ERROR);
       LOG(("Installation directory and working directory must be the same "
            "for non-staged updates. Exiting."));
       LogFinish();
       return 1;
     }