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

[MWB] Fix helper process termination issue in service mode #36892

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

YDKK
Copy link

@YDKK YDKK commented Jan 15, 2025

Summary of the Pull Request

This PR addresses an issue reported in #30259, where running MWB (Mouse Without Borders) in service mode causes the helper process to remain terminated at various points, resulting in functionalities such as clipboard sharing becoming non-functional.

Detailed Description of the Pull Request / Additional comments

When MWB is launched in service mode, events such as desktop switches (e.g., UAC prompts) cause the helper process to be killed and subsequently restarted. To restart the helper process within the user's desktop session, the CreateProcessInInputDesktopSession method uses APIs such as WTSQueryUserToken, which require privileges and the LocalSystem account to function properly.
Link to the relevant code:

internal static int CreateProcessInInputDesktopSession(string commandLine, string arg, string desktop, short wShowWindow, bool lowIntegrity = false)
// As user who runs explorer.exe
{
if (!Program.User.Contains("system", StringComparison.InvariantCultureIgnoreCase))
{
ProcessStartInfo s = new(commandLine, arg);
s.WindowStyle = wShowWindow != 0 ? ProcessWindowStyle.Normal : ProcessWindowStyle.Hidden;
Process p = Process.Start(s);
return p == null ? 0 : p.Id;
}
string commandLineWithArg = commandLine + " " + arg;
int lastError;
int dwSessionId;
IntPtr hUserToken = IntPtr.Zero, hUserTokenDup = IntPtr.Zero;
Logger.LogDebug("CreateProcessInInputDesktopSession called, launching " + commandLineWithArg + " on " + desktop);
try
{
dwSessionId = Process.GetCurrentProcess().SessionId;
// Get the user token used by DuplicateTokenEx
lastError = (int)NativeMethods.WTSQueryUserToken((uint)dwSessionId, ref hUserToken);
NativeMethods.STARTUPINFO si = default;
si.cb = Marshal.SizeOf(si);
si.lpDesktop = "winsta0\\" + desktop;
si.wShowWindow = wShowWindow;
NativeMethods.SECURITY_ATTRIBUTES sa = default;
sa.Length = Marshal.SizeOf(sa);
if (!NativeMethods.DuplicateTokenEx(hUserToken, NativeMethods.MAXIMUM_ALLOWED, ref sa, (int)NativeMethods.SECURITY_IMPERSONATION_LEVEL.SecurityIdentification, (int)NativeMethods.TOKEN_TYPE.TokenPrimary, ref hUserTokenDup))
{
lastError = Marshal.GetLastWin32Error();
Logger.Log(string.Format(CultureInfo.CurrentCulture, "DuplicateTokenEx error: {0} Token does not have the privilege.", lastError));
_ = NativeMethods.CloseHandle(hUserToken);
return 0;
}
if (lowIntegrity)
{
NativeMethods.TOKEN_MANDATORY_LABEL tIL;
// Low
string sIntegritySid = "S-1-16-4096";
bool rv = NativeMethods.ConvertStringSidToSid(sIntegritySid, out IntPtr pIntegritySid);
if (!rv)
{
Logger.Log("ConvertStringSidToSid failed");
_ = NativeMethods.CloseHandle(hUserToken);
_ = NativeMethods.CloseHandle(hUserTokenDup);
return 0;
}
tIL.Label.Attributes = NativeMethods.SE_GROUP_INTEGRITY;
tIL.Label.Sid = pIntegritySid;
rv = NativeMethods.SetTokenInformation(hUserTokenDup, NativeMethods.TOKEN_INFORMATION_CLASS.TokenIntegrityLevel, ref tIL, (uint)Marshal.SizeOf(tIL) + (uint)IntPtr.Size);
if (!rv)
{
Logger.Log("SetTokenInformation failed");
_ = NativeMethods.CloseHandle(hUserToken);
_ = NativeMethods.CloseHandle(hUserTokenDup);
return 0;
}
}
uint dwCreationFlags = NativeMethods.NORMAL_PRIORITY_CLASS | NativeMethods.CREATE_NEW_CONSOLE;
IntPtr pEnv = IntPtr.Zero;
if (NativeMethods.CreateEnvironmentBlock(ref pEnv, hUserTokenDup, true))
{
dwCreationFlags |= NativeMethods.CREATE_UNICODE_ENVIRONMENT;
}
else
{
pEnv = IntPtr.Zero;
}
_ = NativeMethods.CreateProcessAsUser(
hUserTokenDup, // client's access token
null, // file to execute
commandLineWithArg, // command line
ref sa, // pointer to process SECURITY_ATTRIBUTES
ref sa, // pointer to thread SECURITY_ATTRIBUTES
false, // handles are not inheritable
(int)dwCreationFlags, // creation flags
pEnv, // pointer to new environment block
null, // name of current directory
ref si, // pointer to STARTUPINFO structure
out NativeMethods.PROCESS_INFORMATION pi); // receives information about new process
// GetLastError should be 0
int iResultOfCreateProcessAsUser = Marshal.GetLastWin32Error();
Logger.LogDebug("CreateProcessAsUser returned " + iResultOfCreateProcessAsUser.ToString(CultureInfo.CurrentCulture));
// Close handles task
_ = NativeMethods.CloseHandle(hUserToken);
_ = NativeMethods.CloseHandle(hUserTokenDup);
return (iResultOfCreateProcessAsUser == 0) ? (int)pi.dwProcessId : 0;
}
catch (Exception e)
{
Logger.Log(e);
return 0;
}
}

However, in the scenario where this issue occurs, the main thread is impersonated with user-level permissions, causing these API calls to fail. As a result, the helper process remains terminated, and features like clipboard sharing stop working.

Through debugging, I found that the execution permissions of the main thread are impersonated (and not reverted) after a delegate is invoked via DoSomethingInUIThread from other threads (e.g., InputCallback Thread). The root cause of this behavior is unknown at this time, but I personally feel like it's due to a framework-side issue rather than an implementation issue.

After experimenting with several approaches, I discovered that suppressing the flow of the execution context in the calling thread (using ExecutionContext.SuppressFlow();) before invoking DoSomethingInUIThread prevents this issue. Given that suppressing the execution context flow from non-main threads does not appear to pose any problems in the current MWB implementation, this PR applies this fix.

Validation Steps Performed

  1. Launch MWB in service mode and move the mouse cursor.
  2. Trigger desktop switch events such as displaying a UAC prompt or pressing Ctrl+Alt+Delete.
  3. After returning to the original desktop, confirm that the helper process is restarted and that clipboard sharing functionality continues to operate as expected.

@YDKK
Copy link
Author

YDKK commented Jan 15, 2025

@microsoft-github-policy-service agree

@YDKK YDKK marked this pull request as ready for review January 15, 2025 17:17
@YDKK YDKK force-pushed the feature/fix-helper-process-exit branch from 5639d15 to ce41453 Compare January 18, 2025 05:16
Copy link
Collaborator

@jaimecbernardo jaimecbernardo left a comment

Choose a reason for hiding this comment

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

Thanks for opening this pull request. Very interesting investigation 😄
I've merged latest main and merged it in, since it caused a build error with a recent refactor.
I've given it a try and it seems to fix it on my Windows 10 machine when going through Ctrl+Alt+Delete. On Windows 11, the Helper process never gets restarted.
On both Windows 10 and Windows 11, after going through the Lock Screen and having to login again, the Helper process doesn't get restarted.
So I think this fix is still incomplete. Can you please take a look? Thanks, in advance.

@YDKK
Copy link
Author

YDKK commented Jan 20, 2025

@jaimecbernardo
Thanks for your checking and feedback!

Hmm, that's weird...
I checked again on the branch after merging.
I'm on Windows 11 24H2 and I can confirm that the helper process restarts fine, including through the Lock Screen.

Screen record of the helper process restarting through the Lock Screen

capture
Can confirm that the helper process was restarted from PID 63468 to 31516.

Is there any additional context needed to reproduce?

Thanks,

@jaimecbernardo
Copy link
Collaborator

@YDKK , Gave it another try. After just starting, indeed it's working well, but after a couple of copy pastes from one side to the other and then going to Ctrl-Alt-Del screen, after I cancel that screen Helper doesn't come back.

Trying to get a more detailed repro here's what I did.
Setup:
PC1 is on Windows 11 and PC2 is on Windows 10 (not sure if relevant). Both are connected through MouseWIthoutBorders, both are Using Service and PowerToys is running as admin.
1- On PC1, I press Ctrl+Alt+Del and then I press Cancel. Helper starts again.
2 - Using PC1's keyboard and mouse, I copy a string from PC1 notepad++ using Ctrl+C and paste it into PC2 notepad++ using Ctrl+V.
3 - On PC1, I press Ctrl+Alt+Del and then I press Cancel. Helper starts.
4 - Using PC1's keyboard and mouse, I copy a string from PC2 notepad++ using Ctrl+C and paste it into PC2 notepad++ using Ctrl+V.
5 - On PC1, I press Ctrl+Alt+Del and then I press Cancel. Helper doesn't start.

Having pasted on the PC I press Ctrl+Alt+Del on seems to be causing this issue? 🤔
I've tried again running as service but without PowerToys running as admin and the same happens.

@YDKK
Copy link
Author

YDKK commented Jan 21, 2025

@jaimecbernardo
Thanks for the detailed steps.
I was able to reproduce the problem myself.

After further investigation, I found that the fixes for the threads generated by sockets and clipboard related were missing, so I fixed them in the same way.

I'm sorry to bother you, but could you check again?

@YDKK YDKK requested a review from jaimecbernardo January 21, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants