mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-20 14:06:30 +00:00
fix: state the size of every read the DACL walk makes
The null guards did not clear the analyzer, and on reflection they were not the whole answer. The dereference read one byte through a pointer a foreign function had written into an out-parameter, with the size of the read implied by the type being cast to rather than stated anywhere. The ACE header is now copied out as a fixed-size block. The bytes read are the same bytes; what changes is that the amount is written down at the point of the read, which is the property worth having in the one function that decides whether the lock directory can be trusted. Whether this satisfies the analyzer is not yet known - its model treats a pointer written through a foreign out-parameter as unvalidated regardless of what the callee's contract promises, and that is a fair position for it to take about code it cannot see into. If the alert survives, the remaining options are the owner's rather than mine: dismissing it in the security tab is a repo-admin action, and I am not going to weaken a security read to quiet a warning about it.
This commit is contained in:
parent
215589591e
commit
7581a1fa64
1 changed files with 9 additions and 1 deletions
|
|
@ -95,6 +95,10 @@ fn sid_len(sid: &SidBuffer) -> usize {
|
|||
/// pointer rather than a proof of it. The read is the security check that
|
||||
/// decides whether the lock directory is trustworthy, so it fails closed on a
|
||||
/// pointer it cannot justify instead of dereferencing one.
|
||||
///
|
||||
/// The header is copied out as a fixed-size block rather than read field by
|
||||
/// field through the foreign pointer, so the size of every read is stated at
|
||||
/// the call site instead of being implied by the type being dereferenced.
|
||||
pub(super) fn dacl_grants_only(acl: *const ACL, accepted: &[&SidBuffer]) -> std::io::Result<bool> {
|
||||
if acl.is_null() {
|
||||
return Err(std::io::Error::from(std::io::ErrorKind::InvalidInput));
|
||||
|
|
@ -108,7 +112,11 @@ pub(super) fn dacl_grants_only(acl: *const ACL, accepted: &[&SidBuffer]) -> std:
|
|||
if ace_ptr.is_null() {
|
||||
return Err(std::io::Error::from(std::io::ErrorKind::InvalidData));
|
||||
}
|
||||
let ace_type = unsafe { *ace_ptr.cast::<u8>() };
|
||||
let mut header = [0_u8; ACE_FIXED_LEN];
|
||||
unsafe {
|
||||
std::ptr::copy_nonoverlapping(ace_ptr.cast::<u8>(), header.as_mut_ptr(), header.len());
|
||||
}
|
||||
let ace_type = header[0];
|
||||
if ace_type != ACCESS_ALLOWED_ACE_TYPE {
|
||||
return Ok(false);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue