Skip to content

Commit

Permalink
Revert to using the metainstaller for elevating machine handoff installs
Browse files Browse the repository at this point in the history
The previous CL used `GoogleUpdate.exe` for elevating machine installs. However, this made machine installs elevate from the `TMP` directory, which is insecure. This CL reverts to using the metainstaller for elevating machine handoff installs, as it was before.

This CL does keep the setup changes of not copying the metainstaller to the versioned install directory. The metainstaller was previously needed for cross-install scenarios, i.e., a user installation /handoff doing a machine install, where elevating the entire metainstaller was safer. However, that scenario was only used by the ActiveX/plugin code, and is no longer in use.
  • Loading branch information
sorinj committed May 5, 2023
1 parent 72f9bdd commit b68ee97
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 17 deletions.
7 changes: 3 additions & 4 deletions omaha/client/install.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,10 @@ HRESULT ElevateAndWait(const CString& cmd_line, DWORD* exit_code) {
SafeCStringAppendFormat(&cmd_line_elevated, _T(" /%s"),
kCmdLineInstallElevated);

HRESULT hr = goopdate_utils::StartElevatedSelfWithArgsAndWait(
cmd_line_elevated, exit_code);
HRESULT hr = goopdate_utils::StartElevatedMetainstaller(cmd_line_elevated,
exit_code);
if (FAILED(hr)) {
OPT_LOG(LE,
(_T("[Elevated GoogleUpdate.exe failed][%s][%#x]"), cmd_line, hr));
OPT_LOG(LE, (_T("[Elevated metainstaller failed][%s][%#x]"), cmd_line, hr));

// TODO(omaha3): Report hr somehow. Was reported in extra code in Omaha 2.
if (vista_util::IsUserNonElevatedAdmin()) {
Expand Down
32 changes: 21 additions & 11 deletions omaha/common/goopdate_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -308,29 +308,39 @@ CString BuildGoogleUpdateServicesEnclosedPath(bool is_machine, bool use64bit) {
return path;
}

HRESULT StartElevatedSelfWithArgsAndWait(const TCHAR* args, DWORD* exit_code) {
HRESULT StartElevatedMetainstaller(const TCHAR* args, DWORD* exit_code) {
ASSERT1(args);
ASSERT1(exit_code);
CORE_LOG(L3, (_T("[StartElevatedMetainstaller]")));

CORE_LOG(L3, (_T("[StartElevatedSelfWithArgsAndWait]")));
CPath metainstaller_path(app_util::GetCurrentModuleDirectory());
VERIFY1(metainstaller_path.Append(kOmahaMetainstallerFileName));
if (!File::Exists(metainstaller_path)) {
return GOOPDATE_E_METAINSTALLER_NOT_FOUND;
}

// Get the process executable.
const CString filename(app_util::GetModulePath(NULL));
// In case the metainstaller has been tagged, append the /no_mi_tag switch
// to the end of the command line to signal the MI to not append its tag.
// The MI will remove the switch before launching the constant shell.
// TODO(omaha): Once the ApplyTag library has been refactored to allow
// removal/truncation of tags, remove the tag as part of setup (so that the
// MI in the permanent install is always untagged) and eliminate this switch.
CString new_args;
SafeCStringFormat(&new_args, _T("%s /%s"), args, kCmdLineNoMetainstallerTag);

// Launch self elevated and wait.
// Launch metainstaller elevated and wait.
*exit_code = 0;
CORE_LOG(L3, (_T("[Elevating][%s][%s]"), filename, args));
CORE_LOG(L3, (_T("[Elevating][%s][%s]"), metainstaller_path, new_args));

// According to the MSDN documentation for ::ShowWindow: "nCmdShow. This
// parameter is ignored the first time an application calls ShowWindow, if
// the program that launched the application provides a STARTUPINFO
// structure.". We want to force showing the UI window. So we pass in
// SW_SHOWNORMAL.
HRESULT hr =
vista_util::RunElevated(filename,
args,
SW_SHOWNORMAL,
exit_code);
HRESULT hr = vista_util::RunElevated(metainstaller_path,
new_args,
SW_SHOWNORMAL,
exit_code);
CORE_LOG(L3, (_T("[elevated instance exit code][%u]"), *exit_code));

if (FAILED(hr)) {
Expand Down
5 changes: 3 additions & 2 deletions omaha/common/goopdate_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ HRESULT StartGoogleUpdateWithArgs(bool is_machine,
// if we're running on a 64-bit OS.
HRESULT StartCrashHandler(bool is_machine);

// Runs the current executable in an elevated mode using the "Runas" verb.
HRESULT StartElevatedSelfWithArgsAndWait(const TCHAR* args, DWORD* exit_code);
// Starts the metainstaller in the same directory as the current module in an
// elevated mode using the "Runas" verb.
HRESULT StartElevatedMetainstaller(const TCHAR* args, DWORD* exit_code);

// Registers security and sets the security values for the GoogleUpdate
// process when running as a COM server.
Expand Down

0 comments on commit b68ee97

Please sign in to comment.