stop over-reporting world-writable directories (#6936)

Fix world-writable audit false positives by expanding generic
permissions with MapGenericMask and then checking only concrete write
bits. The earlier check looked for FILE_GENERIC_WRITE/generic masks
directly, which shares bits with read permissions and could flag an
Everyone read ACE as writable.
This commit is contained in:
iceweasel-oai 2025-11-19 13:59:17 -08:00 committed by GitHub
parent 056c8f8279
commit 2fde03b4a0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -9,15 +9,29 @@ use std::path::PathBuf;
use std::time::Duration;
use std::time::Instant;
use windows_sys::Win32::Foundation::CloseHandle;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Foundation::ERROR_SUCCESS;
use windows_sys::Win32::Foundation::HLOCAL;
use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE;
use windows_sys::Win32::Foundation::LocalFree;
use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE;
use windows_sys::Win32::Security::ACE_HEADER;
use windows_sys::Win32::Security::ACL;
use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
use windows_sys::Win32::Security::AclSizeInformation;
use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW;
use windows_sys::Win32::Security::Authorization::GetSecurityInfo;
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
use windows_sys::Win32::Security::EqualSid;
use windows_sys::Win32::Security::GetAce;
use windows_sys::Win32::Security::GetAclInformation;
use windows_sys::Win32::Security::MapGenericMask;
use windows_sys::Win32::Security::GENERIC_MAPPING;
use windows_sys::Win32::Storage::FileSystem::CreateFileW;
use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS;
use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ;
use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE;
use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ;
@ -26,17 +40,6 @@ use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA;
use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA;
use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING;
const GENERIC_ALL_MASK: u32 = 0x1000_0000;
const GENERIC_WRITE_MASK: u32 = 0x4000_0000;
use windows_sys::Win32::Security::AclSizeInformation;
use windows_sys::Win32::Security::EqualSid;
use windows_sys::Win32::Security::GetAce;
use windows_sys::Win32::Security::GetAclInformation;
use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE;
use windows_sys::Win32::Security::ACE_HEADER;
use windows_sys::Win32::Security::ACL;
use windows_sys::Win32::Security::ACL_SIZE_INFORMATION;
use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION;
// Preflight scan limits
const MAX_ITEMS_PER_DIR: i32 = 1000;
@ -304,7 +307,7 @@ pub fn world_writable_warning_details(
}
}
// Fast mask-based check: does the DACL contain any ACCESS_ALLOWED ACE for
// Everyone that includes generic or specific write bits? Skips inherit-only
// Everyone that grants write after generic bits are expanded? Skips inherit-only
// ACEs (do not apply to the current object).
unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut c_void) -> bool {
if p_dacl.is_null() {
@ -321,6 +324,12 @@ unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut
if ok == 0 {
return false;
}
let mapping = GENERIC_MAPPING {
GenericRead: FILE_GENERIC_READ,
GenericWrite: FILE_GENERIC_WRITE,
GenericExecute: FILE_GENERIC_EXECUTE,
GenericAll: FILE_ALL_ACCESS,
};
for i in 0..(info.AceCount as usize) {
let mut p_ace: *mut c_void = std::ptr::null_mut();
if GetAce(p_dacl as *const ACL, i as u32, &mut p_ace) == 0 {
@ -337,19 +346,16 @@ unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut
let base = p_ace as usize;
let sid_ptr =
(base + std::mem::size_of::<ACE_HEADER>() + std::mem::size_of::<u32>()) as *mut c_void; // skip header + mask
if EqualSid(sid_ptr, psid_world) != 0 {
let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE);
let mask = ace.Mask;
let writey = FILE_GENERIC_WRITE
| FILE_WRITE_DATA
| FILE_APPEND_DATA
| FILE_WRITE_EA
| FILE_WRITE_ATTRIBUTES
| GENERIC_WRITE_MASK
| GENERIC_ALL_MASK;
if (mask & writey) != 0 {
return true;
}
if EqualSid(sid_ptr, psid_world) == 0 {
continue;
}
let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE);
let mut mask = ace.Mask;
// Expand generic bits to concrete file rights before checking for write.
MapGenericMask(&mut mask, &mapping);
let write_mask = FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_EA | FILE_WRITE_ATTRIBUTES;
if (mask & write_mask) != 0 {
return true;
}
}
false