Skip to content

Commit

Permalink
Fix Security Elevation of Privilege in Chrome installer during update
Browse files Browse the repository at this point in the history
We use the `new_setup_exe` directory as the working directory for
`ArchivePatchHelper::UncompressAndPatch`. For System installs, this
directory would be under %ProgramFiles% (a directory that only admins
can write to by default) and hence a secure location.

Bug: 1279188
Change-Id: I8f65ff67d588c46d81abc09616a08e19be2820e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3351102
Auto-Submit: S. Ganesh <ganesh@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/main@{#955697}
  • Loading branch information
GitHubGanesh authored and Chromium LUCI CQ committed Jan 5, 2022
1 parent 705ab01 commit 368a755
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 29 deletions.
48 changes: 21 additions & 27 deletions chrome/installer/setup/setup_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -922,33 +922,27 @@ bool HandleNonInstallCmdLineOptions(installer::ModifyParams& modify_params,
// patch to current exe, and store the resulting binary in the path
// specified by --new-setup-exe. But we need to first unpack the file
// given in --update-setup-exe.
base::ScopedTempDir temp_path;
if (!temp_path.CreateUniqueTempDir()) {
PLOG(ERROR) << "Could not create temporary path.";
} else {
base::FilePath compressed_archive(
cmd_line.GetSwitchValuePath(installer::switches::kUpdateSetupExe));
VLOG(1) << "Opening archive " << compressed_archive.value();
// The top unpack failure result with 28 days aggregation (>=0.01%)
// Setup.Install.LzmaUnPackResult_SetupExePatch
// 0.02% PATH_NOT_FOUND
//
// More information can also be found with metric:
// Setup.Install.LzmaUnPackNTSTATUS_SetupExePatch
if (installer::ArchivePatchHelper::UncompressAndPatch(
temp_path.GetPath(), compressed_archive, setup_exe,
cmd_line.GetSwitchValuePath(installer::switches::kNewSetupExe),
installer::UnPackConsumer::SETUP_EXE_PATCH)) {
status = installer::NEW_VERSION_UPDATED;
}
if (!temp_path.Delete()) {
// PLOG would be nice, but Delete() doesn't leave a meaningful value in
// the Windows last-error code.
LOG(WARNING) << "Scheduling temporary path "
<< temp_path.GetPath().value()
<< " for deletion at reboot.";
ScheduleDirectoryForDeletion(temp_path.GetPath());
}

const base::FilePath compressed_archive(
cmd_line.GetSwitchValuePath(installer::switches::kUpdateSetupExe));
VLOG(1) << "Opening archive " << compressed_archive.value();
// The top unpack failure result with 28 days aggregation (>=0.01%)
// Setup.Install.LzmaUnPackResult_SetupExePatch
// 0.02% PATH_NOT_FOUND
//
// More information can also be found with metric:
// Setup.Install.LzmaUnPackNTSTATUS_SetupExePatch

// We use the `new_setup_exe` directory as the working directory for
// `ArchivePatchHelper::UncompressAndPatch`. For System installs, this
// directory would be under %ProgramFiles% (a directory that only admins can
// write to by default) and hence a secure location.
const base::FilePath new_setup_exe(
cmd_line.GetSwitchValuePath(installer::switches::kNewSetupExe));
if (installer::ArchivePatchHelper::UncompressAndPatch(
new_setup_exe.DirName(), compressed_archive, setup_exe,
new_setup_exe, installer::UnPackConsumer::SETUP_EXE_PATCH)) {
status = installer::NEW_VERSION_UPDATED;
}

*exit_code = InstallUtil::GetInstallReturnCode(status);
Expand Down
6 changes: 4 additions & 2 deletions chrome/installer/util/util_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,10 @@ const char kMakeChromeDefault[] = "make-chrome-default";
// Tells installer to expect to be run as a subsidiary to an MSI.
const char kMsi[] = "msi";

// Useful only when used with --update-setup-exe, otherwise ignored. It
// specifies the full path where updated setup.exe will be stored.
// Useful only when used with --update-setup-exe; otherwise ignored. Specifies
// the full path where the updated setup.exe will be written. Any other files
// created in the indicated directory may be deleted by the caller after process
// termination.
const char kNewSetupExe[] = "new-setup-exe";

// Specifies a nonce to use with the rotate device key command.
Expand Down

0 comments on commit 368a755

Please sign in to comment.