Skip to content

Commit

Permalink
refactor(pedm): audit clippy suppressions
Browse files Browse the repository at this point in the history
Issue: DGW-215
  • Loading branch information
CBenoit committed Nov 5, 2024
1 parent 0ffca58 commit af8950b
Show file tree
Hide file tree
Showing 14 changed files with 46 additions and 43 deletions.
3 changes: 0 additions & 3 deletions crates/devolutions-pedm-hook/src/appinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions crates/devolutions-pedm-hook/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions crates/devolutions-pedm-shell-ext/src/lib_win.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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() };
Expand Down
2 changes: 1 addition & 1 deletion crates/devolutions-pedm/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl From<Error> 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,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/devolutions-pedm/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions crates/win-api-wrappers/src/identity/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -190,7 +190,7 @@ impl ProfileInfo {
token,
username: WideString::from(username),
raw: PROFILEINFOW {
dwSize: size_of_u32::<PROFILEINFOW>(),
dwSize: u32size_of::<PROFILEINFOW>(),
dwFlags: PI_NOUI,
..Default::default()
},
Expand Down
6 changes: 3 additions & 3 deletions crates/win-api-wrappers/src/identity/sid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<SID>() }))
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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::<SID>() };

Expand Down
12 changes: 6 additions & 6 deletions crates/win-api-wrappers/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -225,7 +225,7 @@ impl Process {
self.handle.raw(),
ProcessBasicInformation,
&mut basic_info as *mut _ as *mut _,
size_of_u32::<PROCESS_BASIC_INFORMATION>(),
u32size_of::<PROCESS_BASIC_INFORMATION>(),
None,
)
}?;
Expand Down Expand Up @@ -334,7 +334,7 @@ pub fn shell_execute(
let verb = WideString::from(verb);

let mut exec_info = SHELLEXECUTEINFOW {
cbSize: size_of_u32::<SHELLEXECUTEINFOW>(),
cbSize: u32size_of::<SHELLEXECUTEINFOW>(),
fMask: SEE_MASK_NOCLOSEPROCESS,
lpFile: path.as_pcwstr(),
lpParameters: command_line.as_pcwstr(),
Expand Down Expand Up @@ -450,9 +450,9 @@ impl StartupInfo {
Ok(STARTUPINFOEXW {
StartupInfo: STARTUPINFOW {
cb: if self.attribute_list.is_some() {
size_of_u32::<STARTUPINFOEXW>()
u32size_of::<STARTUPINFOEXW>()
} else {
size_of_u32::<STARTUPINFOW>()
u32size_of::<STARTUPINFOW>()
},
lpReserved: self.reserved.as_pwstr(),
lpDesktop: self.desktop.as_pwstr(),
Expand Down Expand Up @@ -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()),
Expand Down
1 change: 0 additions & 1 deletion crates/win-api-wrappers/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
13 changes: 7 additions & 6 deletions crates/win-api-wrappers/src/security/acl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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::<ACE_HEADER>` big.
unsafe {
ptr.cast::<ACE_HEADER>().write(header)
Expand All @@ -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::<ACE_HEADER>()) };

#[allow(clippy::cast_ptr_alignment)]
#[allow(clippy::cast_ptr_alignment)] // FIXME(DGW-221): Raw* hack is flawed.
// SAFETY: Buffer is at least `size_of::<ACE_HEADER> + size_of::<u32>` big.
unsafe {
ptr.cast::<u32>().write(self.access_mask)
Expand Down Expand Up @@ -116,7 +117,7 @@ impl Ace {
ptr = unsafe { ptr.byte_add(mem::size_of::<ACE_HEADER>()) };

// 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::<u32>().read() };

// SAFETY: Assume buffer is big enough to fit Ace data.
Expand Down Expand Up @@ -449,7 +450,7 @@ impl TryFrom<&SecurityAttributes> for RawSecurityAttributes {
.transpose()?;

let raw = SECURITY_ATTRIBUTES {
nLength: size_of_u32::<SECURITY_ATTRIBUTES>(),
nLength: u32size_of::<SECURITY_ATTRIBUTES>(),
lpSecurityDescriptor: security_descriptor
.as_mut()
.map_or_else(ptr::null_mut, |x| x.as_raw_mut() as *mut _ as *mut _),
Expand Down
12 changes: 6 additions & 6 deletions crates/win-api-wrappers/src/security/crypt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -70,21 +70,21 @@ pub fn win_verify_trust(path: &Path, catalog_info: Option<CatalogInfo>) -> Resul

let mut wintrust_info = match &catalog_info {
Some((catalog_info_path, catalog_info_member)) => WintrustInfo::Catalog(WINTRUST_CATALOG_INFO {
cbStruct: size_of_u32::<WINTRUST_CATALOG_INFO>(),
cbStruct: u32size_of::<WINTRUST_CATALOG_INFO>(),
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::<WINTRUST_FILE_INFO>(),
cbStruct: u32size_of::<WINTRUST_FILE_INFO>(),
pcwszFilePath: path.as_pcwstr(),
..Default::default()
}),
};

let mut win_trust_data = WINTRUST_DATA {
cbStruct: size_of_u32::<WINTRUST_DATA>(),
cbStruct: u32size_of::<WINTRUST_DATA>(),
dwUIChoice: WTD_UI_NONE,
fdwRevocationChecks: WTD_REVOKE_WHOLECHAIN,
dwUnionChoice: match &wintrust_info {
Expand Down Expand Up @@ -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::<CATALOG_INFO>(),
cbStruct: u32size_of::<CATALOG_INFO>(),
..Default::default()
};

Expand Down Expand Up @@ -558,7 +558,7 @@ pub fn cert_ctx_eku(ctx: &CERT_CONTEXT) -> Result<Vec<String>> {
}

// 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::<CTL_USAGE>().read() };

Ok(
Expand Down
4 changes: 2 additions & 2 deletions crates/win-api-wrappers/src/security/privilege.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<TOKEN_PRIVILEGES>()
}
Expand All @@ -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::<TOKEN_PRIVILEGES>() };

privileges.PrivilegeCount = value.0.len().try_into()?;
Expand Down
14 changes: 8 additions & 6 deletions crates/win-api-wrappers/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -93,14 +93,14 @@ impl Token {
) -> Result<Self> {
// See https://github.com/decoder-it/CreateTokenExample/blob/master/StopZillaCreateToken.cpp#L344
let sqos = SECURITY_QUALITY_OF_SERVICE {
Length: size_of_u32::<SECURITY_QUALITY_OF_SERVICE>(),
Length: u32size_of::<SECURITY_QUALITY_OF_SERVICE>(),
ImpersonationLevel: SecurityImpersonation,
ContextTrackingMode: SECURITY_DYNAMIC_TRACKING.0,
EffectiveOnly: false.into(),
};

let object_attributes = OBJECT_ATTRIBUTES {
Length: size_of_u32::<OBJECT_ATTRIBUTES>(),
Length: u32size_of::<OBJECT_ATTRIBUTES>(),
SecurityQualityOfService: &sqos as *const _ as *const c_void,
..Default::default()
};
Expand Down Expand Up @@ -241,7 +241,7 @@ impl Token {
self.handle.raw(),
info_class,
Some(info.as_mut_ptr().cast()),
size_of_u32::<T>(),
u32size_of::<T>(),
&mut return_length,
)
}?;
Expand All @@ -257,7 +257,7 @@ impl Token {
self.handle.raw(),
info_class,
value as *const _ as *const _,
size_of_u32::<T>(),
u32size_of::<T>(),
)?;
}

Expand Down Expand Up @@ -480,6 +480,8 @@ impl Token {
}

pub fn logon_sid(&self) -> Result<Sid> {
// 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()?
Expand Down Expand Up @@ -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::<TOKEN_GROUPS>() };

groups.GroupCount = value.0.len().try_into()?;
Expand Down
7 changes: 5 additions & 2 deletions crates/win-api-wrappers/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ impl<'a> ProcessIdIterator<'a> {
snapshot,
is_first: true,
entry: PROCESSENTRY32 {
dwSize: size_of_u32::<PROCESSENTRY32>(),
dwSize: u32size_of::<PROCESSENTRY32>(),
..Default::default()
},
}
Expand Down Expand Up @@ -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<T>() -> u32 {
pub(crate) const fn u32size_of<T>() -> u32 {
mem::size_of::<T>() as u32
}

0 comments on commit af8950b

Please sign in to comment.