From 1c7606cea18e908cf76201ce1534b0afdc04cc89 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 13 Nov 2020 03:10:00 +0100 Subject: manager: allow S-1-5-32-556 users to launch a limited UI I still have serious security reservations about this, both conceptually -- should users be allowed to do this stuff? -- and pratically -- there are issues with this implementation that need some examination. TODO: - Is that registry key a secure path? Should we double check it? - Are we leaking handles to the unpriv'd process from the manager? Audit this too. - IPC notifications are blocking. Should we move this to a go routine to mitigate DoS potential? - Is GOB deserialization secure? Can an NCO user crash or RCE the manager? Signed-off-by: Jason A. Donenfeld --- main.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'main.go') diff --git a/main.go b/main.go index 868be5d0..b8c385fe 100644 --- a/main.go +++ b/main.go @@ -137,10 +137,10 @@ func main() { checkForWow64() if len(os.Args) <= 1 { - checkForAdminGroup() if ui.RaiseUI() { return } + checkForAdminGroup() err := execElevatedManagerServiceInstaller() if err != nil { fatal(err) @@ -213,9 +213,18 @@ func main() { if len(os.Args) != 6 { usage() } - err := elevate.DropAllPrivileges(false) - if err != nil { - fatal(err) + var processToken windows.Token + isAdmin := false + err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY|windows.TOKEN_DUPLICATE, &processToken) + if err == nil { + isAdmin = elevate.TokenIsElevatedOrElevatable(processToken) + processToken.Close() + } + if isAdmin { + err := elevate.DropAllPrivileges(false) + if err != nil { + fatal(err) + } } readPipe, err := pipeFromHandleArgument(os.Args[2]) if err != nil { @@ -234,6 +243,7 @@ func main() { fatal(err) } manager.InitializeIPCClient(readPipe, writePipe, eventPipe) + ui.IsAdmin = isAdmin ui.RunUI() return case "/dumplog": -- cgit v1.2.3-59-g8ed1b