From 6fbfccc2d3a40f848dbe77e858d044cd6dfc6737 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 26 Aug 2024 19:31:21 +0200 Subject: [PATCH] fs: fix race condition in get/set security descriptor Calling `Load()` twice for an atomic variable can return different values each time. This resulted in trying to read the security descriptor with high privileges, but then not entering the code path to switch to low privileges when another thread has already done so concurrently. --- internal/fs/sd_windows.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/fs/sd_windows.go b/internal/fs/sd_windows.go index 0a73cbe53..bccf74992 100644 --- a/internal/fs/sd_windows.go +++ b/internal/fs/sd_windows.go @@ -48,13 +48,15 @@ func GetSecurityDescriptor(filePath string) (securityDescriptor *[]byte, err err var sd *windows.SECURITY_DESCRIPTOR - if lowerPrivileges.Load() { + // store original value to avoid unrelated changes in the error check + useLowerPrivileges := lowerPrivileges.Load() + if useLowerPrivileges { sd, err = getNamedSecurityInfoLow(filePath) } else { sd, err = getNamedSecurityInfoHigh(filePath) } if err != nil { - if !lowerPrivileges.Load() && isHandlePrivilegeNotHeldError(err) { + if !useLowerPrivileges && isHandlePrivilegeNotHeldError(err) { // If ERROR_PRIVILEGE_NOT_HELD is encountered, fallback to backups/restores using lower non-admin privileges. lowerPrivileges.Store(true) sd, err = getNamedSecurityInfoLow(filePath) @@ -109,14 +111,16 @@ func SetSecurityDescriptor(filePath string, securityDescriptor *[]byte) error { sacl = nil } - if lowerPrivileges.Load() { + // store original value to avoid unrelated changes in the error check + useLowerPrivileges := lowerPrivileges.Load() + if useLowerPrivileges { err = setNamedSecurityInfoLow(filePath, dacl) } else { err = setNamedSecurityInfoHigh(filePath, owner, group, dacl, sacl) } if err != nil { - if !lowerPrivileges.Load() && isHandlePrivilegeNotHeldError(err) { + if !useLowerPrivileges && isHandlePrivilegeNotHeldError(err) { // If ERROR_PRIVILEGE_NOT_HELD is encountered, fallback to backups/restores using lower non-admin privileges. lowerPrivileges.Store(true) err = setNamedSecurityInfoLow(filePath, dacl)