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

PerfView should run the debugging application as a normal user and not as an elevated (admin) user #135

Open
CoderParPlaisir opened this issue Nov 1, 2016 · 9 comments

Comments

@CoderParPlaisir
Copy link

Hello

When we run an application with PerfView (clicking on "Run a command", or by the menus "Collect" / "Run" (Alt-R), or by command line "run"), PerfView elevates itself then run the application as elevated too.
That is a security threat, and it prevents debugging Window Store applications (they fail running, see https://social.msdn.microsoft.com/Forums/en-US/41170c0f-405c-45d8-abcd-b7a376c70c48/failure-starting-process-in-perview-with-windows-10-universal-application?forum=wpdevelop).

PerfView, even elevated, should run the application as the normal non-elevated user.

It is obviously feasible as said here:
https://blogs.msdn.microsoft.com/winsdk/2013/06/17/launching-a-process-as-a-normal-user-from-an-elevated-user/
It is even easier in PerfView as when PerfView starts non-elevated, it knows who is the normal user and it can collect the non-elevated token and pass it to the elevated PerfView process.

Thank you.

Tested with PerfView 1.9.0.0

TEST:

  • Run PerView.
  • Alt-R
  • Type-in command to the application.
  • Click on run command.
  • Check in the task manager that the application as been launched with elevated privileges (admin user).

IMPACT:

  • Windows Store applications can not been profiled this way.
  • That is a security threat since the application has admin rights, and usuallly it was not designed for that.
  • The developper can not profile its application as a normal non-elevated user (the behavior of the application can be very different).
@vancem
Copy link
Contributor

vancem commented Nov 2, 2016

I am assuming everyone understands the obvious work-around of simply using the 'Collect' rather than the 'Run' command. If you start PerfView's collection you can then launch your app however you wish, (and in particular non-elevated). This should not block investigations, and thus is a convenience/security feature.

This feature is a bit more complex because you probably want to expose the ability to run the app elevated as well as non-elevated (non-elevated can be the default).

I am generally supportive of this feature. If anyone in the community wishes to tackle it I can provide advice about the details necessary to get it done.

@ppozeti
Copy link

ppozeti commented Oct 2, 2018

Hey @vancem,
What about if we add a checkbox next to the Command text box (RunCommandDialog.xml) and if is checked then the process is started elevated.

@vancem
Copy link
Contributor

vancem commented Oct 2, 2018

@ppozeti - Sure having a checkbox to choose is reasonable. The solution is straightfoward, the issue is the priority. Because it has such a simple work-around it has low priority. If anyone wants to take a crack at it by all means do so.

@ppozeti
Copy link

ppozeti commented Oct 3, 2018

Well, can I do this? I'm just starting at contributing to open source projects and I thought that would be a good start. Could you assign this to me?

@vancem vancem assigned vancem and unassigned vancem Oct 3, 2018
@vancem
Copy link
Contributor

vancem commented Oct 3, 2018

GitHub won't let me actually set the assigned to field to you, but this note indicate that others should not work on it without interacting here.

@brianrob
Copy link
Member

@ppozeti is this something that you're still interested in working on?

@ppozeti
Copy link

ppozeti commented Oct 18, 2019

@ppozeti is this something that you're still interested in working on?

Sure!

@brianrob
Copy link
Member

Excellent. Then I'll keep this issue around. Do post if you have any questions or run into issues. Thanks.

@brianrob brianrob added this to the Backlog milestone Oct 20, 2019
@HMartyrossian
Copy link
Contributor

@ppozeti
Paulo, I'm assuming that you will address this issue for Windows only, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants