From 39e37482d6f884bf4765dbe9c95a93dc2a92e559 Mon Sep 17 00:00:00 2001 From: "GPT 5.4" Date: Sun, 12 Apr 2026 21:05:59 +0800 Subject: [PATCH 1/3] fix: make `gix-sec` identity check on Windows similar to Git for Windows Co-authored-by: Sebastian Thiel --- gix-sec/src/identity.rs | 104 ++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 35 deletions(-) diff --git a/gix-sec/src/identity.rs b/gix-sec/src/identity.rs index 420aa8963ed..cb76bbbce60 100644 --- a/gix-sec/src/identity.rs +++ b/gix-sec/src/identity.rs @@ -63,9 +63,9 @@ mod impl_ { #[cfg(windows)] mod impl_ { use std::{ - io, + io, mem, mem::MaybeUninit, - os::windows::io::{FromRawHandle as _, OwnedHandle}, + os::windows::io::{AsRawHandle as _, FromRawHandle as _, OwnedHandle}, path::Path, ptr, }; @@ -80,14 +80,46 @@ mod impl_ { }}; } + fn token_information(token: windows_sys::Win32::Foundation::HANDLE, class: i32) -> io::Result> { + use windows_sys::Win32::{ + Foundation::{GetLastError, ERROR_INSUFFICIENT_BUFFER}, + Security::GetTokenInformation, + }; + + #[allow(unsafe_code)] + unsafe { + let mut buffer_size = 36; + let mut heap_buf = vec![0; 36]; + + loop { + if GetTokenInformation( + token, + class, + heap_buf.as_mut_ptr().cast(), + heap_buf.len() as _, + &mut buffer_size, + ) != 0 + { + return Ok(heap_buf); + } + + if GetLastError() != ERROR_INSUFFICIENT_BUFFER { + error!("Couldn't acquire token information"); + } + + heap_buf.resize(buffer_size as _, 0); + } + } + } + pub fn is_path_owned_by_current_user(path: &Path) -> io::Result { use windows_sys::Win32::{ - Foundation::{GetLastError, LocalFree, ERROR_INSUFFICIENT_BUFFER, ERROR_INVALID_FUNCTION, ERROR_SUCCESS}, + Foundation::{LocalFree, ERROR_INVALID_FUNCTION, ERROR_SUCCESS}, Security::{ Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT}, - CheckTokenMembership, EqualSid, GetTokenInformation, IsWellKnownSid, TokenOwner, - WinBuiltinAdministratorsSid, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, TOKEN_OWNER, - TOKEN_QUERY, + CheckTokenMembership, EqualSid, IsWellKnownSid, TokenElevationType, TokenElevationTypeLimited, + TokenLinkedToken, TokenUser, WinBuiltinAdministratorsSid, OWNER_SECURITY_INFORMATION, + PSECURITY_DESCRIPTOR, TOKEN_ELEVATION_TYPE, TOKEN_LINKED_TOKEN, TOKEN_QUERY, TOKEN_USER, }, System::Threading::{GetCurrentProcess, GetCurrentThread, OpenProcessToken, OpenThreadToken}, }; @@ -169,46 +201,48 @@ mod impl_ { let _owned_token = OwnedHandle::from_raw_handle(token as _); - let buf = 'token_buf: { - let mut buffer_size = 36; - let mut heap_buf = vec![0; 36]; - - loop { - if GetTokenInformation( - token, - TokenOwner, - heap_buf.as_mut_ptr().cast(), - heap_buf.len() as _, - &mut buffer_size, - ) != 0 - { - break 'token_buf heap_buf; - } + let user_info = token_information(token, TokenUser)?; + let token_user = (*user_info.as_ptr().cast::()).User.Sid; - if GetLastError() != ERROR_INSUFFICIENT_BUFFER { - error!("Couldn't acquire token ownership"); - } + if EqualSid(folder_owner, token_user) != 0 { + return Ok(true); + } - heap_buf.resize(buffer_size as _, 0); - } - }; + // Admin-group owned folders are considered owned by the current user, if they are in the admin group. + if IsWellKnownSid(folder_owner, WinBuiltinAdministratorsSid) == 0 { + return Ok(false); + } - let token_owner = (*buf.as_ptr().cast::()).Owner; + let mut is_member = 0; + if CheckTokenMembership(std::ptr::null_mut(), folder_owner, &mut is_member) == 0 { + error!("Couldn't check if user is an administrator"); + } - // If the current user is the owner of the parent folder then they also - // own this file - if EqualSid(folder_owner, token_owner) != 0 { + if is_member != 0 { return Ok(true); } - // Admin-group owned folders are considered owned by the current user, if they are in the admin group - if IsWellKnownSid(token_owner, WinBuiltinAdministratorsSid) == 0 { + let mut elevation_type = TokenElevationTypeLimited; + let mut elevation_type_size = 0; + if windows_sys::Win32::Security::GetTokenInformation( + token, + TokenElevationType, + (&mut elevation_type as *mut TOKEN_ELEVATION_TYPE).cast(), + mem::size_of::() as u32, + &mut elevation_type_size, + ) == 0 + || elevation_type != TokenElevationTypeLimited + { return Ok(false); } + let linked_token_info = token_information(token, TokenLinkedToken)?; + let linked_token = (*linked_token_info.as_ptr().cast::()).LinkedToken; + let linked_token = OwnedHandle::from_raw_handle(linked_token as _); + let mut is_member = 0; - if CheckTokenMembership(std::ptr::null_mut(), token_owner, &mut is_member) == 0 { - error!("Couldn't check if user is an administrator"); + if CheckTokenMembership(linked_token.as_raw_handle() as _, folder_owner, &mut is_member) == 0 { + error!("Couldn't check if elevated user is an administrator"); } Ok(is_member != 0) From 57106cda3c94544038ded4d103f7b9c3ee354760 Mon Sep 17 00:00:00 2001 From: "GPT 5.4" Date: Sat, 18 Apr 2026 11:05:22 +0800 Subject: [PATCH 2/3] make `gix-sec` trust check failures more obvious on Windows With the added context, it should be easier to tell where the failure is coming from. Co-authored-by: Sebastian Thiel --- gix-sec/src/identity.rs | 45 ++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/gix-sec/src/identity.rs b/gix-sec/src/identity.rs index cb76bbbce60..2ed5a6fdad1 100644 --- a/gix-sec/src/identity.rs +++ b/gix-sec/src/identity.rs @@ -76,11 +76,17 @@ mod impl_ { error!(inner, $msg); }}; ($inner:expr, $msg:expr) => {{ - return Err(io::Error::new($inner.kind(), $msg)); + return Err(io::Error::new($inner.kind(), format!("{}: {}", $msg, $inner))); }}; } - fn token_information(token: windows_sys::Win32::Foundation::HANDLE, class: i32) -> io::Result> { + fn token_information( + token: windows_sys::Win32::Foundation::HANDLE, + class: i32, + class_name: &'static str, + subject: &'static str, + path: &Path, + ) -> io::Result> { use windows_sys::Win32::{ Foundation::{GetLastError, ERROR_INSUFFICIENT_BUFFER}, Security::GetTokenInformation, @@ -104,7 +110,10 @@ mod impl_ { } if GetLastError() != ERROR_INSUFFICIENT_BUFFER { - error!("Couldn't acquire token information"); + error!(format!( + "Couldn't acquire {class_name} for the {subject} while checking ownership of '{}'", + path.display() + )); } heap_buf.resize(buffer_size as _, 0); @@ -163,10 +172,7 @@ mod impl_ { let inner = io::Error::from_raw_os_error(result as _); error!( inner, - format!( - "Couldn't get security information for path '{}' with err {inner}", - path.display() - ) + format!("Couldn't get security information for path '{}'", path.display()) ); } @@ -194,14 +200,17 @@ mod impl_ { if OpenThreadToken(GetCurrentThread(), TOKEN_QUERY, 1, token.as_mut_ptr()) == 0 && OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, token.as_mut_ptr()) == 0 { - error!("Couldn't acquire thread or process token"); + error!(format!( + "Couldn't acquire a thread or process token while checking ownership of '{}'", + path.display() + )); } token.assume_init() }; let _owned_token = OwnedHandle::from_raw_handle(token as _); - let user_info = token_information(token, TokenUser)?; + let user_info = token_information(token, TokenUser, "TokenUser", "current token", path)?; let token_user = (*user_info.as_ptr().cast::()).User.Sid; if EqualSid(folder_owner, token_user) != 0 { @@ -215,7 +224,10 @@ mod impl_ { let mut is_member = 0; if CheckTokenMembership(std::ptr::null_mut(), folder_owner, &mut is_member) == 0 { - error!("Couldn't check if user is an administrator"); + error!(format!( + "Couldn't check whether the current token is in the Administrators group while checking ownership of '{}'", + path.display() + )); } if is_member != 0 { @@ -236,13 +248,22 @@ mod impl_ { return Ok(false); } - let linked_token_info = token_information(token, TokenLinkedToken)?; + let linked_token_info = token_information( + token, + TokenLinkedToken, + "TokenLinkedToken", + "limited current token", + path, + )?; let linked_token = (*linked_token_info.as_ptr().cast::()).LinkedToken; let linked_token = OwnedHandle::from_raw_handle(linked_token as _); let mut is_member = 0; if CheckTokenMembership(linked_token.as_raw_handle() as _, folder_owner, &mut is_member) == 0 { - error!("Couldn't check if elevated user is an administrator"); + error!(format!( + "Couldn't check whether the linked elevated token is in the Administrators group while checking ownership of '{}'", + path.display() + )); } Ok(is_member != 0) From 8a18a7d79ce60cc3192cb8804c28b9740d73f546 Mon Sep 17 00:00:00 2001 From: "GPT 5.4" Date: Sat, 18 Apr 2026 11:13:05 +0800 Subject: [PATCH 3/3] address auto-review - gix-sec/src/identity.rs:213 still reads TOKEN_USER out of a Vec via pointer cast and dereference. That is the alignment/UB issue Copilot and Codex flagged. - gix-sec/src/identity.rs:251 does the same for TOKEN_LINKED_TOKEN. Co-authored-by: Sebastian Thiel --- gix-sec/src/identity.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gix-sec/src/identity.rs b/gix-sec/src/identity.rs index 2ed5a6fdad1..c3224074f39 100644 --- a/gix-sec/src/identity.rs +++ b/gix-sec/src/identity.rs @@ -211,7 +211,8 @@ mod impl_ { let _owned_token = OwnedHandle::from_raw_handle(token as _); let user_info = token_information(token, TokenUser, "TokenUser", "current token", path)?; - let token_user = (*user_info.as_ptr().cast::()).User.Sid; + let user_info = ptr::read_unaligned(user_info.as_ptr().cast::()); + let token_user = user_info.User.Sid; if EqualSid(folder_owner, token_user) != 0 { return Ok(true); @@ -255,7 +256,8 @@ mod impl_ { "limited current token", path, )?; - let linked_token = (*linked_token_info.as_ptr().cast::()).LinkedToken; + let linked_token_info = ptr::read_unaligned(linked_token_info.as_ptr().cast::()); + let linked_token = linked_token_info.LinkedToken; let linked_token = OwnedHandle::from_raw_handle(linked_token as _); let mut is_member = 0;