Unable to drag/drop a file to terminal with UAC turned off #15757

@zadjii-msft commented on GitHub (Nov 1, 2021):

To be totally sure I'm doing this right - what's the output of reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua?

@zadjii-msft commented on GitHub (Nov 1, 2021): To be totally sure I'm doing this right - what's the output of `reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua`?

claunia commented

@d0j commented on GitHub (Nov 1, 2021):

To be totally sure I'm doing this right - what's the output of reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua?

`reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua

HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System
enablelua REG_DWORD 0x0`

@d0j commented on GitHub (Nov 1, 2021): > To be totally sure I'm doing this right - what's the output of `reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua`? `reg query HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System\ /v enablelua HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System enablelua REG_DWORD 0x0`

claunia commented

@zadjii-msft commented on GitHub (Nov 1, 2021):

Huh okay, that's the situation that I thought I fixed. Weird.

@zadjii-msft commented on GitHub (Nov 1, 2021): Huh okay, that's the situation that I thought I fixed. Weird.

claunia commented

@eryksun commented on GitHub (Dec 16, 2021):

I can confirm that it isn't fixed in 1.12.2931.0. I disabled UAC and logged on with an administrator account. I verified that the access token of Explorer in this case has high mandatory integrity level and has the administrators group enabled, the same as Terminal.

@eryksun commented on GitHub (Dec 16, 2021): I can confirm that it isn't fixed in 1.12.2931.0. I disabled UAC and logged on with an administrator account. I verified that the access token of Explorer in this case has high mandatory integrity level and has the administrators group enabled, the same as Terminal.

claunia commented

@eryksun commented on GitHub (Dec 16, 2021):

IMO, IsElevated() from GH-11221 is badly named. The name is misleading because the function returns false for default elevation. It also checks for administrator access, which is separate from the token's elevation level. It should be named something descriptive like IsUserALinkedAdmin().

For the elevation check, it only needs to check for the elevation type TokenElevationTypeFull, which implies that the token is an elevated token that's linked to a limited token.

The function also has a technical bug, but one that's not an issue currently. Using nullptr for the token passed to test_token_membership() uses the effective token of the current thread in the underlying CheckTokenMembership() call. But the elevation check explicitly uses the process token, i.e. GetCurrentProcessToken() (i.e. (HANDLE)(LONG_PTR)-4). For the effective token, use GetCurrentThreadEffectiveToken() (i.e. (HANDLE)(LONG_PTR)-6), which references the thread token, if the thread is impersonating, else the process token. A name like IsUserALinkedAdmin() implies it should be checking the process token in both cases. I don't think Terminal ever impersonates another user, so there's no need to worry about effective access. BTW, closing these pseudohandles is a wasted system call that does nothing. There's no need for RAII.

@eryksun commented on GitHub (Dec 16, 2021): IMO, `IsElevated()` from GH-11221 is badly named. The name is misleading because the function returns false for default elevation. It also checks for administrator access, which is separate from the token's elevation level. It should be named something descriptive like `IsUserALinkedAdmin()`. For the elevation check, it only needs to check for the elevation type `TokenElevationTypeFull`, which implies that the token is an elevated token that's linked to a limited token. The function also has a technical bug, but one that's not an issue currently. Using `nullptr` for the token passed to `test_token_membership()` uses the effective token of the current thread in the underlying `CheckTokenMembership()` call. But the elevation check explicitly uses the process token, i.e. `GetCurrentProcessToken()` (i.e. `(HANDLE)(LONG_PTR)-4`). For the effective token, use `GetCurrentThreadEffectiveToken()` (i.e. `(HANDLE)(LONG_PTR)-6`), which references the thread token, if the thread is impersonating, else the process token. A name like `IsUserALinkedAdmin()` implies it should be checking the process token in both cases. I don't think Terminal ever impersonates another user, so there's no need to worry about effective access. BTW, closing these pseudohandles is a wasted system call that does nothing. There's no need for RAII.

claunia commented

@DHowett commented on GitHub (Dec 16, 2021):

FWIW I believe that the implementation in IsElevated as of 1.12 does the wrong thing as well, but in a different direction.

@DHowett commented on GitHub (Dec 16, 2021): FWIW I believe that the implementation in `IsElevated` _as of 1.12_ does the wrong thing as well, but in a different direction.

claunia commented

@zadjii-msft commented on GitHub (Jan 6, 2022):

Huh, so observation: I don't think we can fix this currently. Seems like the OS straight up disables XAML drag/drop when EnableLUA is set to 0. I'm trying it out with Sticky Notes in a VM, and that's not letting me drag/drop anything into it. I don't think the TermControl is doing anything to suppress the Drag event - it doesn't reject drops when it's elevated:
a766798fb8/src/cascadia/TerminalControl/TermControl.cpp (L2373-L2392)

@zadjii-msft commented on GitHub (Jan 6, 2022): Huh, so observation: I don't think we _can_ fix this currently. Seems like the OS straight up disables XAML drag/drop when EnableLUA is set to 0. I'm trying it out with Sticky Notes in a VM, and that's not letting me drag/drop _anything_ into it. I don't think the TermControl is doing anything to suppress the Drag event - it doesn't reject drops when it's elevated: https://github.com/microsoft/terminal/blob/a766798fb829a9e6f91706e919b71feabcb9b35e/src/cascadia/TerminalControl/TermControl.cpp#L2373-L2392

claunia commented

@zadjii-msft commented on GitHub (Jan 6, 2022):

MSFT:35616520 has more context here. Looks like this is going to be an unfortunate "no" for now. We may need to move to a future version of WinAppSDK that might support this, whenever support for elevated drag/drop is added to WASDK.

@zadjii-msft commented on GitHub (Jan 6, 2022): MSFT:35616520 has more context here. Looks like this is going to be an unfortunate "no" for now. We may need to move to a future version of WinAppSDK that might support this, whenever support for elevated drag/drop is added to WASDK.

claunia commented

@DHowett commented on GitHub (Jan 6, 2022):

Wow, I really don't know what I meant when I said that "1.12 had it wrong in the opposite direction" -- the code is the same, as far as I can tell.

The core issue that prevents tab reordering is that we have a process, DataExchangeHost, that is used to render the bitmap attached to the cursor and shuttle data into the receiving process. That process must be running as the same user and at the same IL (I think¹) as Terminal.

The two cases where it isn't running as the same user or at the same IL are:

Linked token?
Over-the-shoulder elevation. This might secretly work (?) because of how brokering works.

Since we can't detect the identity of the DataExchangeHost, we had to solve for a heuristic . . . so IsElevated was originally designed to handle (1) because we needed to use it to disable drag/drop in case (1).

It was probably a mistake to try to generalize it and use it for anything else -- and we may want to rectify that mistake.

However, the issue IN THIS THREAD is different. Terminal doesn't directly control what external applications can drag/drop to it -- this is just a platform limitation. We'll need to take it up internally.

¹ Since it's just COM, and it doesn't want to "punch down" at a lower-IL process. I think.

@DHowett commented on GitHub (Jan 6, 2022): Wow, I really don't know what I meant when I said that "1.12 had it wrong in the opposite direction" -- the code is the same, as far as I can tell. The core issue that prevents tab reordering is that we have a process, DataExchangeHost, that is used to render the bitmap attached to the cursor and shuttle data into the receiving process. That process *must be running as the same user* and *at the same IL* (I think¹) as Terminal. The two cases where it isn't running as the same user or at the same IL are: 1. Linked token? 2. Over-the-shoulder elevation. This might secretly work (?) because of how brokering works. Since we can't detect the identity of the DataExchangeHost, we had to solve for a heuristic . . . so IsElevated was originally designed to handle (1) because we needed to _use it to disable drag/drop in case (1)._ It was probably a mistake to try to generalize it and use it for anything else -- and we may want to rectify that mistake. However, the issue IN THIS THREAD is different. Terminal doesn't directly control what external applications can drag/drop to it -- this is just a platform limitation. We'll need to take it up internally. ¹ Since it's just COM, and it doesn't want to "punch down" at a lower-IL process. I think.

claunia commented

@zadjii-msft commented on GitHub (Mar 4, 2022):

Oh hey this is actually tracked elsewhere: /dup #6661

@zadjii-msft commented on GitHub (Mar 4, 2022): Oh hey this is actually tracked elsewhere: /dup #6661

claunia commented

@ghost commented on GitHub (Mar 4, 2022):

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost commented on GitHub (Mar 4, 2022): Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

claunia commented

@he852100 commented on GitHub (Jul 8, 2022):

@zadjii-msft They are two different problems

@he852100 commented on GitHub (Jul 8, 2022): @zadjii-msft They are two different problems ![拖动](https://user-images.githubusercontent.com/26913892/178065760-2965bda9-e8c2-4b9a-8e92-d650e048b1c2.gif)

claunia commented

@a657938016 commented on GitHub (Oct 11, 2022):

@zadjii-毫秒它们是两个不同的问题

麻烦问下你的这个问题解决了吗？我也遇到同样的问题

@a657938016 commented on GitHub (Oct 11, 2022): > @zadjii-毫秒它们是两个不同的问题 ![拖动](https://user-images.githubusercontent.com/26913892/178065760-2965bda9-e8c2-4b9a-8e92-d650e048b1c2.gif) [ ![拖动](https://user-images.githubusercontent.com/26913892/178065760-2965bda9-e8c2-4b9a-8e92-d650e048b1c2.gif) ](https://user-images.githubusercontent.com/26913892/178065760-2965bda9-e8c2-4b9a-8e92-d650e048b1c2.gif) [ ](https://user-images.githubusercontent.com/26913892/178065760-2965bda9-e8c2-4b9a-8e92-d650e048b1c2.gif) 麻烦问下你的这个问题解决了吗？我也遇到同样的问题

claunia commented