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

UIforETW shortcut doesn't launch from within a folder named 'UI for ETW' #147

Closed
eugenesvk opened this issue Jan 31, 2021 · 5 comments · May be fixed by #148
Closed

UIforETW shortcut doesn't launch from within a folder named 'UI for ETW' #147

eugenesvk opened this issue Jan 31, 2021 · 5 comments · May be fixed by #148

Comments

@eugenesvk
Copy link

A UIforETW.lnk shortcut created from a ...\etwpackage\bin\UIforETW.exe file doesn't lanch the UIforETW app if this shortcut is placed in a folder named UI for ETW — the shortcut fails right after asking for elevated permissions.
The same shortcut placed in a different folder (or if the same folder is renamed) works just fine

Windows 10.0.19042.0
etwpackage1.54.zip

@randomascii
Copy link
Contributor

Thanks for the report. This bug is kind of hilarious. At first I thought that the spaces in the folder were the problem, but they aren't. Well, not really. That is, if you remove the spaces then the problem goes away, but it also goes away if you just remove one space, or if you add more.

The problem is that UIforETW looks for a previously running copy of itself and activates itself if it finds one. And it does that with this code:

HWND prevWindow = FindWindow(NULL, L"UI for ETW");
if (prevWindow)
{
	// Only allow one copy to be running at a time.
	SetForegroundWindow(prevWindow);
}

In your scenario the name of the Explorer window is "UI for ETW" so the explorer window is found and activated.

I should be using a clearly unique mutex or some-such, or else checking what process owns the window, or something. Anyway, that's the problem.

Pull requests welcome, or else I'll get around to fixing it at some point. Thanks again.

@randomascii
Copy link
Contributor

Link to the problematic code, which was there in the very first commit:

https://github.com/google/UIforETW/blob/main/UIforETW/UIforETW.cpp#L194

@randomascii
Copy link
Contributor

In addition to the obvious workaround of renaming the folder you can also workaround this by configuring explorer to display full path names in window titles.

Fixing this will be easy, but I don't know when I'll do a new release.

randomascii added a commit that referenced this issue Feb 3, 2021
UIforETW used to look for a window with the title UI for ETW to see if
it was already running, but this can accidentally find the wrong window.
Using a named mutex is more robust - safe against anything but a
malicious local attacker.

With this change UIforETW will reliably launch/not-launch based on
whether it is already running. In not-launch case it may not reliably
activate the correct window but that is a low-severity edge case.

This fixes issue #147
@randomascii
Copy link
Contributor

This fix will ship in the next release.

eugenesvk added a commit to eugenesvk/UIforETW that referenced this issue Feb 8, 2021
Old FindWindow() function could mistake an Explorer folder 'UI for ETW'
for another instance of 'UI for ETW' (since window title is identical)
New custom function registers a unique window message that allows to
uniquely identify another instance of 'UI for ETW'
Bonus: restore the minimized window of 'UI for ETW'

Resolves: google#147
@eugenesvk
Copy link
Author

I've added an extra fix that would not switch to an open folder with the same name, but uniquely identify only the duplicate UIforETW instance via a unique window message.
This should fully cover this issue

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 a pull request may close this issue.

2 participants