From 6937fc8436d978a7c46591d77827e61420785922 Mon Sep 17 00:00:00 2001 From: Sorin Jianu Date: Wed, 31 May 2023 17:04:28 -0700 Subject: [PATCH] Fix Security Omaha EoP on Windows via COM This CL prevents elevation of privilege by reverting to the process token before starting processes. Otherwise, a lower privilege token could for instance symlink `C:\` to a different folder (per-user DosDevice) and allow an elevation of privilege attack. --- omaha/base/error.h | 1 + omaha/base/scoped_impersonation.h | 11 ++++++++++- omaha/base/system.cc | 13 +++++++++++++ omaha/base/utils.cc | 8 ++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/omaha/base/error.h b/omaha/base/error.h index 2172cd0..02fe334 100644 --- a/omaha/base/error.h +++ b/omaha/base/error.h @@ -36,6 +36,7 @@ namespace omaha { // #define EXCEPTION_IMPERSONATION_FAILED 0x1 #define EXCEPTION_REVERT_IMPERSONATION_FAILED 0x2 +#define EXCEPTION_FAILED_TO_GET_THREAD_TOKEN 0x3 // // HRESULT Functions. diff --git a/omaha/base/scoped_impersonation.h b/omaha/base/scoped_impersonation.h index 6a78061..f62a410 100644 --- a/omaha/base/scoped_impersonation.h +++ b/omaha/base/scoped_impersonation.h @@ -94,7 +94,16 @@ struct scoped_impersonation { class scoped_revert_to_self { public: scoped_revert_to_self() { - token_.GetThreadToken(TOKEN_ALL_ACCESS); + if (!token_.GetThreadToken(TOKEN_ALL_ACCESS)) { + if (::GetLastError() != ERROR_NO_TOKEN) { + ::RaiseException(EXCEPTION_FAILED_TO_GET_THREAD_TOKEN, + EXCEPTION_NONCONTINUABLE, + 0, + NULL); + } + return; + } + if (token_.GetHandle()) { RevertToSelfOrDie(); } diff --git a/omaha/base/system.cc b/omaha/base/system.cc index 2c03f6e..221dfe0 100644 --- a/omaha/base/system.cc +++ b/omaha/base/system.cc @@ -31,6 +31,7 @@ #include "omaha/base/path.h" #include "omaha/base/safe_format.h" #include "omaha/base/scope_guard.h" +#include "omaha/base/scoped_impersonation.h" #include "omaha/base/string.h" #include "omaha/base/system_info.h" #include "omaha/base/utils.h" @@ -222,6 +223,12 @@ HRESULT System::StartProcessWithEnvironment( ASSERT1(command_line || process_name); ASSERT(!process_name, (_T("Avoid using process_name. See method comment."))); + // Prevents elevation of privilege by reverting to the process token before + // starting the process. Otherwise, a lower privilege token could for instance + // symlink `C:\` to a different folder (per-user DosDevice) and allow an + // elevation of privilege attack. + scoped_revert_to_self revert_to_self; + STARTUPINFO si = {sizeof(si), 0}; // Feedback cursor is off while the process is starting. @@ -341,6 +348,12 @@ HRESULT System::StartProcessAsUserWithEnvironment( executable_path, parameters, desktop)); ASSERT1(pi); + // Prevents elevation of privilege by reverting to the process token before + // starting the process. Otherwise, a lower privilege token could for instance + // symlink `C:\` to a different folder (per-user DosDevice) and allow an + // elevation of privilege attack. + scoped_revert_to_self revert_to_self; + CString cmd(executable_path); EnclosePath(&cmd); cmd.AppendChar(_T(' ')); diff --git a/omaha/base/utils.cc b/omaha/base/utils.cc index f2f7787..ca81fbf 100644 --- a/omaha/base/utils.cc +++ b/omaha/base/utils.cc @@ -38,6 +38,7 @@ #include "omaha/base/process.h" #include "omaha/base/safe_format.h" #include "omaha/base/scope_guard.h" +#include "omaha/base/scoped_impersonation.h" #include "omaha/base/shell.h" #include "omaha/base/string.h" #include "omaha/base/system.h" @@ -1768,6 +1769,13 @@ bool ShellExecuteExEnsureParent(LPSHELLEXECUTEINFO shell_exec_info) { UTIL_LOG(L3, (_T("[ShellExecuteExEnsureParent]"))); ASSERT1(shell_exec_info); + + // Prevents elevation of privilege by reverting to the process token before + // starting the process. Otherwise, a lower privilege token could for instance + // symlink `C:\` to a different folder (per-user DosDevice) and allow an + // elevation of privilege attack. + scoped_revert_to_self revert_to_self; + bool shell_exec_succeeded(false); DWORD last_error(ERROR_SUCCESS); -- 2.39.1.windows.1