aboutsummaryrefslogtreecommitdiffstatshomepage
path: root/service/service_manager.go
diff options
context:
space:
mode:
authorJason A. Donenfeld <Jason@zx2c4.com>2019-05-11 22:13:31 +0200
committerJason A. Donenfeld <Jason@zx2c4.com>2019-05-12 11:13:35 +0200
commit7fe35e8c15f4c10a5ccaeaa0864ece39469daea6 (patch)
tree72eb403c8d0ef420414e1b55501f0ebcb4afbd22 /service/service_manager.go
parentservice: run UI at high integrity (diff)
downloadwireguard-windows-7fe35e8c15f4c10a5ccaeaa0864ece39469daea6.tar.xz
wireguard-windows-7fe35e8c15f4c10a5ccaeaa0864ece39469daea6.zip
service: run UI with elevated token
There are too many attacks possible when starting this with a non-elevated token. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Diffstat (limited to 'service/service_manager.go')
-rw-r--r--service/service_manager.go33
1 files changed, 10 insertions, 23 deletions
diff --git a/service/service_manager.go b/service/service_manager.go
index c1416c49..c61bd517 100644
--- a/service/service_manager.go
+++ b/service/service_manager.go
@@ -90,26 +90,24 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
if err != nil {
return
}
- defer userToken.Close()
if !tokenIsMemberOfBuiltInAdministrator(userToken) {
+ userToken.Close()
return
}
user, err := userToken.GetTokenUser()
if err != nil {
log.Printf("Unable to lookup user from token: %v", err)
+ userToken.Close()
return
}
username, domain, accType, err := user.User.Sid.LookupAccount("")
if err != nil {
log.Printf("Unable to lookup username from sid: %v", err)
+ userToken.Close()
return
}
if accType != windows.SidTypeUser {
- return
- }
- env, err := userEnviron(userToken)
- if err != nil {
- log.Printf("Unable to determine user environment: %v", err)
+ userToken.Close()
return
}
userTokenInfo := &UserTokenInfo{}
@@ -119,24 +117,15 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
return
}
if userTokenInfo.elevatedToken != userToken {
- defer userTokenInfo.elevatedToken.Close()
+ userToken.Close()
}
- userTokenInfo.elevatedEnvironment, err = userEnviron(userTokenInfo.elevatedToken)
+ defer userTokenInfo.elevatedToken.Close()
+ userToken = 0
+ userTokenInfo.elevatedEnvironment, err = userEnviron(userTokenInfo.elevatedToken) //TODO: This seems to have the same PATH as the userToken. Aren't there attacks?
if err != nil {
log.Printf("Unable to determine elevated environment: %v", err)
return
}
- currentProcess, _ := windows.GetCurrentProcess()
- securityAttributes, err := getSecurityAttributes(windows.Token(currentProcess), userToken)
- if err != nil {
- log.Printf("Unable to extract security attributes from manager token and combine them with SID from user token: %v", err)
- return
- }
- err = addElevatedIntegrityToUserToken(userTokenInfo.elevatedToken, userToken)
- if err != nil {
- log.Printf("Unable to copy integrity level from elevated token to user token")
- return
- }
first := true
for {
if stoppingManager {
@@ -180,12 +169,10 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
log.Printf("Starting UI process for user '%s@%s' for session %d", username, domain, session)
attr := &os.ProcAttr{
Sys: &syscall.SysProcAttr{
- Token: syscall.Token(userToken),
- ProcessAttributes: sliceToSecurityAttributes(securityAttributes),
- ThreadAttributes: sliceToSecurityAttributes(securityAttributes),
+ Token: syscall.Token(userTokenInfo.elevatedToken),
},
Files: []*os.File{devNull, devNull, devNull},
- Env: env,
+ Env: userTokenInfo.elevatedEnvironment,
}
procsLock.Lock()
var proc *os.Process