Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Launch elevated instances via shell:AppFolder #14637

Merged
9 commits merged into from
Jan 19, 2023
Merged
73 changes: 64 additions & 9 deletions src/cascadia/ElevateShim/elevate-shim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
#include <wil/stl.h>
#include <wil/resource.h>
#include <wil/win32_helpers.h>
#include <gsl/gsl_util>
#include <gsl/pointers>
#include <shellapi.h>
#include <appmodel.h>

// BODGY
//
Expand All @@ -25,18 +28,68 @@
// process can successfully elevate.

#pragma warning(suppress : 26461) // we can't change the signature of wWinMain
int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR pCmdLine, int)
int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
{
// All of the args passed to us (something like `new-tab -p {guid}`) are in
// pCmdLine
// This will invoke an elevated terminal in two possible ways. See GH#14501
// In both scenarios, it passes the entire cmdline as-is to the new process.
//
// #1 discover and invoke the app using the GetCurrentApplicationUserModelId
// api using shell:AppsFolder\package!appid
// cmd: shell:AppsFolder\WindowsTerminalDev_8wekyb3d8bbwe!App
// params: new-tab -p {guid}
//
// #2 find and execute WindowsTerminal.exe
// cmd: {same path as this binary}\WindowsTerminal.exe
// params: new-tab -p {guid}

// Get the path to WindowsTerminal.exe, which should live next to us.
std::filesystem::path module{ wil::GetModuleFileNameW<std::wstring>(nullptr) };
// Swap elevate-shim.exe for WindowsTerminal.exe
module.replace_filename(L"WindowsTerminal.exe");
// see if we're a store app we can invoke with shell:AppsFolder
std::wstring appUserModelId;
const auto result = wil::AdaptFixedSizeToAllocatedResult<std::wstring, APPLICATION_USER_MODEL_ID_MAX_LENGTH>(
appUserModelId, [&](PWSTR value, size_t valueLength, gsl::not_null<size_t*> valueLengthNeededWithNull) noexcept -> HRESULT {
UINT32 length = gsl::narrow_cast<UINT32>(valueLength);
const LONG rc = GetCurrentApplicationUserModelId(&length, value);
switch (rc)
{
case ERROR_SUCCESS:
*valueLengthNeededWithNull = length;
return S_OK;

case ERROR_INSUFFICIENT_BUFFER:
*valueLengthNeededWithNull = length;
return S_FALSE; // trigger allocation loop

case APPMODEL_ERROR_NO_APPLICATION:
return E_FAIL; // we are not running as a store app

default:
return E_UNEXPECTED;
}
});
LOG_IF_FAILED(result);

std::wstring cmd = {};
if (result == S_OK && appUserModelId.length() > 0)
{
// scenario #1
cmd = L"shell:AppsFolder\\" + appUserModelId;
}
else
{
// scenario #2
// Get the path to WindowsTerminal.exe, which should live next to us.
std::filesystem::path module{
wil::GetModuleFileNameW<std::wstring>(nullptr)
};
// Swap elevate-shim.exe for WindowsTerminal.exe
module.replace_filename(L"WindowsTerminal.exe");
cmd = module;
}

// Go!

// The cmdline argument passed to WinMain is stripping the first argument.
// Using GetCommandLine() instead for lParameters

// disable warnings from SHELLEXECUTEINFOW struct. We can't fix that.
#pragma warning(push)
#pragma warning(disable : 26476) // Macro uses naked union over variant.
Expand All @@ -46,8 +99,10 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR pCmdLine, int)
seInfo.cbSize = sizeof(seInfo);
seInfo.fMask = SEE_MASK_DEFAULT;
seInfo.lpVerb = L"runas"; // This asks the shell to elevate the process
seInfo.lpFile = module.c_str(); // This is `...\WindowsTerminal.exe`
seInfo.lpParameters = pCmdLine; // This is `new-tab -p {guid}`
seInfo.lpFile = cmd.c_str(); // This is `shell:AppsFolder\...` or `...\WindowsTerminal.exe`
seInfo.lpParameters = GetCommandLine(); // This is `new-tab -p {guid}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the previous pCmdLine?

Copy link
Contributor Author

@jboelter jboelter Jan 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found an innocuous bug in how the first parameter, new-tab, was dropped when passed to elevate-shim.exe by wWinMain given how it's launched by CreateProcess. It appears wWinMain is assuming param zero is the executable path even if it's not.

However, the elevate-shim process is launched with a command line that doesn't include the executable path.

auto cmdline{
fmt::format(L"new-tab {}", newTerminalArgs.ToCommandline().c_str())
};
wil::unique_process_information pi;
STARTUPINFOW si{};
si.cb = sizeof(si);
LOG_IF_WIN32_BOOL_FALSE(CreateProcessW(exePath.c_str(),
cmdline.data(),
nullptr,
nullptr,
FALSE,
0,
nullptr,
nullptr,
&si,
&pi));


This is the !peb from elevate-shim.exe when launched from a dev build of terminal as an appx (I setup Windbg Preview as the post-mortem debugger and put a DebugBreak() at the top of elevate-shim to break in easily). Notice the CommandLine is missing the executable filepath as the first argument. You can also see this in Process Monitor.

    CurrentDirectory:  'C:\windows\system32\'
    WindowTitle:  'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX\elevate-shim.exe'
    ImageFile:    'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX\elevate-shim.exe'
    CommandLine:  '  new-tab --profile "{0caa0dad-35be-5f56-a8ff-afceeeaa6101}"'
    DllPath:      'C:\source\github.com.jboelter.terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX;C:\Program Files\WindowsApps\Microsoft.VCLibs.140.00.Debug.UWPDesktop_14.0.30704.0_x64__8wekyb3d8bbwe;'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeeeeeeeep that's totally on me. We probably could fix that down in TerminalPage.cpp, but 🤷

seInfo.nShow = SW_SHOWNORMAL;
LOG_IF_WIN32_BOOL_FALSE(ShellExecuteExW(&seInfo));

return 0;
}