terminal icon indicating copy to clipboard operation
terminal copied to clipboard

Run as Admin shortcut + elevate:true -> crash loop

Open ShawnXie01 opened this issue 2 years ago • 5 comments

Windows Terminal version

1.14.2281.0 or preview 1.15.2282.0

Windows build number

10.0.19044.0

Other Software

No response

Steps to reproduce

  1. Install terminal regardless preview version or not.
  2. Turn on the "Run this profile as Administrator" in Profile->default
  3. Restart the terminal.

Expected Behavior

No response

Actual Behavior

https://user-images.githubusercontent.com/53345636/188541035-ff4005a1-dbfb-4ed7-8c34-3e9f8367c068.mp4

I also tried run terminal as administrator,but it seemed didn't work.

ShawnXie01 avatar Sep 06 '22 03:09 ShawnXie01

Just xlinking a few issues:

  • #12369
  • #13675
  • #13908

Something weird is going on, for sure

zadjii-msft avatar Sep 06 '22 11:09 zadjii-msft

Just xlinking a few issues:

Something weird is going on, for sure

I read them before I raised the issue.I logged in as my admin user,the only user on my windows,and tried to install Terminal from the MicrosoftStore,yet it still didn't work.

ShawnXie01 avatar Sep 07 '22 08:09 ShawnXie01

Oh man, you know what, I think another thread might have a better repro. Copying deets here.

If a shortcut properties are set to run wt.exe as Admin AND Run as Admin is also set in wt app settings, new tab or the app will not start, and DesktopWindowXamlSource continuously flashes in taskbar.

Terminal ver = 1.15.2874.0 Windows ver = 10.0.19044

zadjii-msft avatar Oct 24 '22 19:10 zadjii-msft

Repro'd a version of the crash loop with the default Administrator account. The issue stems from the way IsElevated() is checking for elevation.

edit: these instructions reproduce the bug; I'm not aware of a fix.

  1. Enable the default Administrator account on your machine (this account is special)
  2. Log into the account (by default no password was set); I used the "Switch User" feature (note: you probably want to set a password and/or disable the account later)
  3. Launch Terminal and change the settings to include a default elevate=true for all profiles
  4. Exit Terminal
  5. Launch Terminal (now w/ all profiles default to elevate); observe an infinite loop while it repeatedly tries to launch an elevated instance from the current instance.
image

Change the default profile settings

    "profiles": 
    {
        "defaults": { "elevate": true },
    ...

Root Cause

I built a custom dev build with some extra logging in the utils.cpp IsElevated check. The built-in Administrator account meets the criteria that it has a TokenElevationTypeDefault token and TokenIsElevated is 1. This in turn causes IsElevated() to return false.

This interacts with TerminalPage::ShouldImmediatelyHandoffToElevated which knows it wants to launch an elevated profile, but (mistakenly) thinks it is not currently elevated. This causes an infinite loop of WindowsTerminal.exe -> elevate-shim.exe while it tries to get an elevated process.

In short, the check for TokenElevationTypeDefault short circuits the elevation check to false and creates an infinite loop.

https://github.com/microsoft/terminal/blob/main/src/types/utils.cpp#L664-L673

            if (elevationType == TokenElevationTypeDefault && elevationState.TokenIsElevated)
            {
                // In this case, the user has UAC entirely disabled. This is sort of
                // weird, we treat this like the user isn't an admin at all. There's no
                // separation of powers, so the things we normally want to gate on
                // "having special powers" doesn't apply.
                //
                // See GH#7754, GH#11096
                return false;
            }

Related Docs:

TokenElevationTypeDefault: Type 1 is a full token with no privileges removed or groups disabled. A full token is only used if User Account Control is disabled or if the user is the built-in Administrator account (for which UAC disabled by default), service account or local system account.

jboelter avatar Jan 08 '23 23:01 jboelter

procmon trace (screenshot) of the loop on latest 1.16.10261.0. This is running under the default Administrator account on 10.0.22621.1105 with elevate=true in the defaults.

The sequence doesn't look right given some lost events, but the PID parent/child relationships are as expected.

WindowsTerminal.exe doesn't think it's elevated (per comment above); it's launching elevate-shim which is using shell:Appsfolder with the runas verb to launch WindowsTerminal elevated and the cycle repeats.

image

jboelter avatar Jan 30 '23 08:01 jboelter

                return false;

Ah, heck. This came up in an earlier code review that we didn't end up merging, too.

We need separate checks for IsUserAnAdminInAWayThatBreaksSomeOfOurFeatures and IsUserAnAdminAtAll. Names TBD. @zadjii-msft, do you remember when that came up? Was it for elevated-state?

DHowett avatar Jan 31 '23 18:01 DHowett

image

zadjii-msft avatar Mar 03 '23 15:03 zadjii-msft