From af8950b591f0c18aa439af90df06f0ac5ee953b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Tue, 5 Nov 2024 23:30:48 +0900 Subject: [PATCH] refactor(pedm): audit clippy suppressions Issue: DGW-215 --- crates/devolutions-pedm-hook/src/appinfo.rs | 3 --- crates/devolutions-pedm-hook/src/lib.rs | 5 +++-- crates/devolutions-pedm-shell-ext/src/lib_win.rs | 4 ++-- crates/devolutions-pedm/src/error.rs | 2 +- crates/devolutions-pedm/src/utils.rs | 2 +- crates/win-api-wrappers/src/identity/account.rs | 4 ++-- crates/win-api-wrappers/src/identity/sid.rs | 6 +++--- crates/win-api-wrappers/src/process.rs | 12 ++++++------ crates/win-api-wrappers/src/rpc.rs | 1 - crates/win-api-wrappers/src/security/acl.rs | 13 +++++++------ crates/win-api-wrappers/src/security/crypt.rs | 12 ++++++------ crates/win-api-wrappers/src/security/privilege.rs | 4 ++-- crates/win-api-wrappers/src/token.rs | 14 ++++++++------ crates/win-api-wrappers/src/utils.rs | 7 +++++-- 14 files changed, 46 insertions(+), 43 deletions(-) diff --git a/crates/devolutions-pedm-hook/src/appinfo.rs b/crates/devolutions-pedm-hook/src/appinfo.rs index 18a05e515..5c72821a3 100644 --- a/crates/devolutions-pedm-hook/src/appinfo.rs +++ b/crates/devolutions-pedm-hook/src/appinfo.rs @@ -16,14 +16,12 @@ use win_api_wrappers::rpc::RpcServerInterfacePointer; /// https://github.com/hfiref0x/UACME/blob/master/Source/Akagi/appinfo/appinfo.idl #[repr(C)] -#[allow(non_snake_case, non_camel_case_types)] pub struct MONITOR_POINT { pub MonitorLeft: u32, pub MonitorRight: u32, } #[repr(C)] -#[allow(non_snake_case, non_camel_case_types)] pub struct APP_STARTUP_INFO { pub Title: PCWSTR, pub X: u32, @@ -39,7 +37,6 @@ pub struct APP_STARTUP_INFO { } #[repr(C)] -#[allow(non_snake_case, non_camel_case_types)] pub struct APP_PROCESS_INFORMATION { pub ProcessHandle: HANDLE, pub ThreadHandle: HANDLE, diff --git a/crates/devolutions-pedm-hook/src/lib.rs b/crates/devolutions-pedm-hook/src/lib.rs index c9b84b30c..c67eff89f 100644 --- a/crates/devolutions-pedm-hook/src/lib.rs +++ b/crates/devolutions-pedm-hook/src/lib.rs @@ -1,6 +1,8 @@ #[cfg(target_os = "windows")] #[path = ""] mod lib_win { + #![allow(non_snake_case, non_camel_case_types)] // WinAPI naming. + pub mod appinfo; pub mod hook; @@ -78,8 +80,7 @@ mod lib_win { } #[no_mangle] - #[allow(non_snake_case, unused_variables)] - extern "system" fn DllMain(dll_module: HINSTANCE, call_reason: u32, _: *mut ()) -> bool { + extern "system" fn DllMain(_dll_module: HINSTANCE, call_reason: u32, _: *mut ()) -> bool { let status = match call_reason { DLL_PROCESS_ATTACH => { thread::spawn(|| match hook() { diff --git a/crates/devolutions-pedm-shell-ext/src/lib_win.rs b/crates/devolutions-pedm-shell-ext/src/lib_win.rs index 259352788..74260aadb 100644 --- a/crates/devolutions-pedm-shell-ext/src/lib_win.rs +++ b/crates/devolutions-pedm-shell-ext/src/lib_win.rs @@ -1,3 +1,5 @@ +#![allow(non_snake_case)] // WinAPI naming. + use std::ffi::{c_void, OsString}; use std::os::windows::ffi::OsStringExt; use std::path::{Path, PathBuf}; @@ -287,7 +289,6 @@ enum ChannelCommand { } #[no_mangle] -#[allow(non_snake_case)] extern "system" fn DllMain(_dll_module: HINSTANCE, call_reason: u32, _: *mut ()) -> bool { match call_reason { DLL_PROCESS_ATTACH => { @@ -336,7 +337,6 @@ impl IClassFactory_Impl for ElevationContextMenuCommandFactory_Impl { } #[no_mangle] -#[allow(non_snake_case)] extern "system" fn DllGetClassObject(class_id: *const GUID, iid: *const GUID, out: *mut *mut c_void) -> HRESULT { // SAFETY: We assume the argument is the correct type according to the doc. let class_id = unsafe { class_id.as_ref() }; diff --git a/crates/devolutions-pedm/src/error.rs b/crates/devolutions-pedm/src/error.rs index 91381575c..078dc1a90 100644 --- a/crates/devolutions-pedm/src/error.rs +++ b/crates/devolutions-pedm/src/error.rs @@ -63,7 +63,7 @@ impl From for ErrorResponse { let win32_error = match kind { Error::AccessDenied => ERROR_ACCESS_DISABLED_BY_POLICY.0, Error::InvalidParameter | Error::NotFound => ERROR_INVALID_PARAMETER.0, - #[allow(clippy::cast_sign_loss)] + #[allow(clippy::cast_sign_loss)] // E_UNEXPECTED fits in a u32. Error::Internal => E_UNEXPECTED.0 as u32, Error::Cancelled => ERROR_CANCELLED.0, }; diff --git a/crates/devolutions-pedm/src/utils.rs b/crates/devolutions-pedm/src/utils.rs index 4d78fc9cc..46193a41a 100644 --- a/crates/devolutions-pedm/src/utils.rs +++ b/crates/devolutions-pedm/src/utils.rs @@ -22,7 +22,7 @@ use win_api_wrappers::utils::{create_directory, CommandLine}; use anyhow::Result; -// WinAPI is verbose, so are we. +// WinAPI's functions have many arguments, we wrap the same way. #[allow(clippy::too_many_arguments)] pub(crate) fn start_process( token: &Token, diff --git a/crates/win-api-wrappers/src/identity/account.rs b/crates/win-api-wrappers/src/identity/account.rs index 6974d9ca6..9cf2f0e9a 100644 --- a/crates/win-api-wrappers/src/identity/account.rs +++ b/crates/win-api-wrappers/src/identity/account.rs @@ -10,7 +10,7 @@ use crate::undoc::{ LsaManageSidNameMapping, LsaSidNameMappingOperation_Add, LSA_SID_NAME_MAPPING_OPERATION_ADD_INPUT, LSA_SID_NAME_MAPPING_OPERATION_GENERIC_OUTPUT, }; -use crate::utils::{size_of_u32, WideString}; +use crate::utils::{u32size_of, WideString}; use crate::Error; use windows::core::{PCWSTR, PWSTR}; use windows::Win32::Foundation::{ERROR_INVALID_SID, MAX_PATH, WIN32_ERROR}; @@ -190,7 +190,7 @@ impl ProfileInfo { token, username: WideString::from(username), raw: PROFILEINFOW { - dwSize: size_of_u32::(), + dwSize: u32size_of::(), dwFlags: PI_NOUI, ..Default::default() }, diff --git a/crates/win-api-wrappers/src/identity/sid.rs b/crates/win-api-wrappers/src/identity/sid.rs index 93a23004e..cea10de7b 100644 --- a/crates/win-api-wrappers/src/identity/sid.rs +++ b/crates/win-api-wrappers/src/identity/sid.rs @@ -73,7 +73,7 @@ impl Sid { buf.truncate(out_size as usize); - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. // SAFETY: We can safely dereference since this is our buffer. We assume the data in the buffer is a SID. Ok(Self::from(unsafe { &*buf.as_ptr().cast::() })) } @@ -165,7 +165,7 @@ impl RawSid { } pub fn as_raw(&self) -> &SID { - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. // SAFETY: Dereferencing is possible since it is our buffer. // We assume our buffer is well constructed and valid. unsafe { @@ -244,7 +244,7 @@ impl TryFrom<&Sid> for RawSid { .size() ]; - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. // SAFETY: Buffer is valid and can fit `SID` and all sub authorites. let sid = unsafe { &mut *buf.as_mut_ptr().cast::() }; diff --git a/crates/win-api-wrappers/src/process.rs b/crates/win-api-wrappers/src/process.rs index 09e171daf..07c9862ac 100644 --- a/crates/win-api-wrappers/src/process.rs +++ b/crates/win-api-wrappers/src/process.rs @@ -40,7 +40,7 @@ use windows::Win32::UI::Shell::{ShellExecuteExW, SEE_MASK_NOCLOSEPROCESS, SHELLE use windows::Win32::UI::WindowsAndMessaging::SHOW_WINDOW_CMD; use super::security::privilege::ScopedPrivileges; -use super::utils::{size_of_u32, ComContext}; +use super::utils::{u32size_of, ComContext}; use windows::Win32::System::Diagnostics::ToolHelp::{ CreateToolhelp32Snapshot, Process32FirstW, Process32NextW, PROCESSENTRY32W, TH32CS_SNAPPROCESS, }; @@ -225,7 +225,7 @@ impl Process { self.handle.raw(), ProcessBasicInformation, &mut basic_info as *mut _ as *mut _, - size_of_u32::(), + u32size_of::(), None, ) }?; @@ -334,7 +334,7 @@ pub fn shell_execute( let verb = WideString::from(verb); let mut exec_info = SHELLEXECUTEINFOW { - cbSize: size_of_u32::(), + cbSize: u32size_of::(), fMask: SEE_MASK_NOCLOSEPROCESS, lpFile: path.as_pcwstr(), lpParameters: command_line.as_pcwstr(), @@ -450,9 +450,9 @@ impl StartupInfo { Ok(STARTUPINFOEXW { StartupInfo: STARTUPINFOW { cb: if self.attribute_list.is_some() { - size_of_u32::() + u32size_of::() } else { - size_of_u32::() + u32size_of::() }, lpReserved: self.reserved.as_pwstr(), lpDesktop: self.desktop.as_pwstr(), @@ -536,7 +536,7 @@ impl Module { // SAFETY: No preconditions. Both handle and symbol are valid. match unsafe { GetProcAddress(self.handle, symbol.as_pcstr()) } { - // Function pointer is wanted. + // This cast is intended. See also: https://github.com/rust-lang/rust-clippy/issues/12638 #[allow(clippy::fn_to_numeric_cast_any)] Some(func) => Ok(func as *const c_void), None => Err(windows::core::Error::from_win32()), diff --git a/crates/win-api-wrappers/src/rpc.rs b/crates/win-api-wrappers/src/rpc.rs index 5c1c55c03..36d4e55be 100644 --- a/crates/win-api-wrappers/src/rpc.rs +++ b/crates/win-api-wrappers/src/rpc.rs @@ -19,7 +19,6 @@ use crate::Error; use anyhow::{bail, Result}; -#[allow(non_camel_case_types)] pub type RPC_BINDING_HANDLE = *mut c_void; pub struct RpcBindingHandle(pub RPC_BINDING_HANDLE); diff --git a/crates/win-api-wrappers/src/security/acl.rs b/crates/win-api-wrappers/src/security/acl.rs index 536641d03..8283125cb 100644 --- a/crates/win-api-wrappers/src/security/acl.rs +++ b/crates/win-api-wrappers/src/security/acl.rs @@ -5,7 +5,7 @@ use std::{ptr, slice}; use anyhow::{bail, Result}; use crate::identity::sid::{RawSid, Sid}; -use crate::utils::{size_of_u32, WideString}; +use crate::utils::{u32size_of, WideString}; use crate::Error; use windows::Win32::Foundation::{ERROR_INVALID_DATA, ERROR_INVALID_VARIANT, E_POINTER}; use windows::Win32::Security::Authorization::{SetNamedSecurityInfoW, SE_OBJECT_TYPE}; @@ -25,8 +25,9 @@ pub enum AceType { impl AceType { pub fn kind(&self) -> u8 { + // ACE type is actually encoded on a u8 even though the type in windows crate is u32. + #[allow(clippy::cast_possible_truncation)] match self { - #[allow(clippy::cast_possible_truncation)] AceType::AccessAllowed(_) => ACCESS_ALLOWED_ACE_TYPE as u8, } } @@ -75,7 +76,7 @@ impl Ace { let mut ptr = buf.as_mut_ptr(); - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. // SAFETY: Buffer is at least `size_of::` big. unsafe { ptr.cast::().write(header) @@ -84,7 +85,7 @@ impl Ace { // SAFETY: We are adding to the pointer in byte aligned mode to access next field. ptr = unsafe { ptr.byte_add(mem::size_of::()) }; - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. // SAFETY: Buffer is at least `size_of:: + size_of::` big. unsafe { ptr.cast::().write(self.access_mask) @@ -116,7 +117,7 @@ impl Ace { ptr = unsafe { ptr.byte_add(mem::size_of::()) }; // SAFETY: Assume that the header is followed by a 4 byte access mask. - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. let access_mask = unsafe { ptr.cast::().read() }; // SAFETY: Assume buffer is big enough to fit Ace data. @@ -449,7 +450,7 @@ impl TryFrom<&SecurityAttributes> for RawSecurityAttributes { .transpose()?; let raw = SECURITY_ATTRIBUTES { - nLength: size_of_u32::(), + nLength: u32size_of::(), lpSecurityDescriptor: security_descriptor .as_mut() .map_or_else(ptr::null_mut, |x| x.as_raw_mut() as *mut _ as *mut _), diff --git a/crates/win-api-wrappers/src/security/crypt.rs b/crates/win-api-wrappers/src/security/crypt.rs index eac20b5e5..4f1a9b55a 100644 --- a/crates/win-api-wrappers/src/security/crypt.rs +++ b/crates/win-api-wrappers/src/security/crypt.rs @@ -7,7 +7,7 @@ use std::path::{Path, PathBuf}; use anyhow::{anyhow, bail, Result}; -use crate::utils::{nul_slice_wide_str, size_of_u32, slice_from_ptr, SafeWindowsString, WideString}; +use crate::utils::{nul_slice_wide_str, slice_from_ptr, u32size_of, SafeWindowsString, WideString}; use crate::Error; use windows::core::HRESULT; use windows::Win32::Foundation::{ @@ -70,21 +70,21 @@ pub fn win_verify_trust(path: &Path, catalog_info: Option) -> Resul let mut wintrust_info = match &catalog_info { Some((catalog_info_path, catalog_info_member)) => WintrustInfo::Catalog(WINTRUST_CATALOG_INFO { - cbStruct: size_of_u32::(), + cbStruct: u32size_of::(), pcwszCatalogFilePath: catalog_info_path.as_pcwstr(), pcwszMemberFilePath: path.as_pcwstr(), pcwszMemberTag: catalog_info_member.as_pcwstr(), ..Default::default() }), None => WintrustInfo::File(WINTRUST_FILE_INFO { - cbStruct: size_of_u32::(), + cbStruct: u32size_of::(), pcwszFilePath: path.as_pcwstr(), ..Default::default() }), }; let mut win_trust_data = WINTRUST_DATA { - cbStruct: size_of_u32::(), + cbStruct: u32size_of::(), dwUIChoice: WTD_UI_NONE, fdwRevocationChecks: WTD_REVOKE_WHOLECHAIN, dwUnionChoice: match &wintrust_info { @@ -245,7 +245,7 @@ impl Iterator for CatalogIterator<'_> { self.cur = Some(HANDLE(new_ctx as *mut c_void)); let mut info = CATALOG_INFO { - cbStruct: size_of_u32::(), + cbStruct: u32size_of::(), ..Default::default() }; @@ -558,7 +558,7 @@ pub fn cert_ctx_eku(ctx: &CERT_CONTEXT) -> Result> { } // SAFETY: We assume `CertGetEnhancedKeyUsage` actually wrote a valid `CTL_USAGE`. - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. let ctl_usage = unsafe { raw_buf.as_ptr().cast::().read() }; Ok( diff --git a/crates/win-api-wrappers/src/security/privilege.rs b/crates/win-api-wrappers/src/security/privilege.rs index a914fe0d8..897af64d4 100644 --- a/crates/win-api-wrappers/src/security/privilege.rs +++ b/crates/win-api-wrappers/src/security/privilege.rs @@ -38,7 +38,7 @@ impl TryFrom<&TOKEN_PRIVILEGES> for TokenPrivileges { impl RawTokenPrivileges { pub fn as_raw(&self) -> &TOKEN_PRIVILEGES { // SAFETY: It is safe to dereference since it is our buffer. - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. unsafe { &*self.0.as_ptr().cast::() } @@ -59,7 +59,7 @@ impl TryFrom<&TokenPrivileges> for RawTokenPrivileges { ]; // SAFETY: `buf` is at least as big as `TOKEN_PRIVILEGES` and its privileges. - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. let privileges = unsafe { &mut *buf.as_mut_ptr().cast::() }; privileges.PrivilegeCount = value.0.len().try_into()?; diff --git a/crates/win-api-wrappers/src/token.rs b/crates/win-api-wrappers/src/token.rs index fe5576516..671191a00 100644 --- a/crates/win-api-wrappers/src/token.rs +++ b/crates/win-api-wrappers/src/token.rs @@ -24,7 +24,7 @@ use crate::undoc::{ TOKEN_SECURITY_ATTRIBUTE_TYPE_OCTET_STRING, TOKEN_SECURITY_ATTRIBUTE_TYPE_STRING, TOKEN_SECURITY_ATTRIBUTE_TYPE_UINT64, TOKEN_SECURITY_ATTRIBUTE_V1, TOKEN_SECURITY_ATTRIBUTE_V1_VALUE, }; -use crate::utils::{size_of_u32, WideString}; +use crate::utils::{u32size_of, WideString}; use crate::{create_impersonation_context, Error}; use windows::Win32::Foundation::{ ERROR_ALREADY_EXISTS, ERROR_INVALID_SECURITY_DESCR, ERROR_INVALID_VARIANT, ERROR_NO_TOKEN, ERROR_SUCCESS, HANDLE, @@ -93,14 +93,14 @@ impl Token { ) -> Result { // See https://github.com/decoder-it/CreateTokenExample/blob/master/StopZillaCreateToken.cpp#L344 let sqos = SECURITY_QUALITY_OF_SERVICE { - Length: size_of_u32::(), + Length: u32size_of::(), ImpersonationLevel: SecurityImpersonation, ContextTrackingMode: SECURITY_DYNAMIC_TRACKING.0, EffectiveOnly: false.into(), }; let object_attributes = OBJECT_ATTRIBUTES { - Length: size_of_u32::(), + Length: u32size_of::(), SecurityQualityOfService: &sqos as *const _ as *const c_void, ..Default::default() }; @@ -241,7 +241,7 @@ impl Token { self.handle.raw(), info_class, Some(info.as_mut_ptr().cast()), - size_of_u32::(), + u32size_of::(), &mut return_length, ) }?; @@ -257,7 +257,7 @@ impl Token { self.handle.raw(), info_class, value as *const _ as *const _, - size_of_u32::(), + u32size_of::(), )?; } @@ -480,6 +480,8 @@ impl Token { } pub fn logon_sid(&self) -> Result { + // Here, losing the sign is fine, because we want to make a bitwise comparison (g.attributes & SE_GROUP_LOGON_ID). + // TODO: Use `cast_unsigned` / `cast_signed` when stabilized: https://github.com/rust-lang/rust/issues/125882 #[allow(clippy::cast_sign_loss)] Ok(self .groups()? @@ -569,7 +571,7 @@ impl TryFrom<&TokenGroups> for RawTokenGroups { ]; // SAFETY: `buf` is at least as big as `TOKEN_GROUPS` and its groups. - #[allow(clippy::cast_ptr_alignment)] + #[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed. let groups = unsafe { &mut *buf.as_mut_ptr().cast::() }; groups.GroupCount = value.0.len().try_into()?; diff --git a/crates/win-api-wrappers/src/utils.rs b/crates/win-api-wrappers/src/utils.rs index ea50b7498..9c459ff1d 100644 --- a/crates/win-api-wrappers/src/utils.rs +++ b/crates/win-api-wrappers/src/utils.rs @@ -459,7 +459,7 @@ impl<'a> ProcessIdIterator<'a> { snapshot, is_first: true, entry: PROCESSENTRY32 { - dwSize: size_of_u32::(), + dwSize: u32size_of::(), ..Default::default() }, } @@ -775,7 +775,10 @@ pub fn nul_slice_wide_str(slice: &[u16]) -> &[u16] { &slice[..last_idx] } +/// Like [`std::mem::size_of`], but returns a u32 instead. +/// +/// Typically fine since we rarely work with structs whose size in memory is bigger than u32::MAX. #[allow(clippy::cast_possible_truncation)] -pub(crate) const fn size_of_u32() -> u32 { +pub(crate) const fn u32size_of() -> u32 { mem::size_of::() as u32 }