Skip to content

Commit

Permalink
Fix
Browse files Browse the repository at this point in the history
This commit fixes an oversight that would've resulted in UB, and fixed the previously mentioned "issue" with signatures consisting of zeroes.
  • Loading branch information
Jujstme committed Oct 7, 2024
1 parent 030b767 commit 8af2c3e
Showing 1 changed file with 43 additions and 21 deletions.
64 changes: 43 additions & 21 deletions src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,59 +199,81 @@ impl<const N: usize> Signature<N> {
let overall_end = addr.value() + len;

// The sigscan essentially works by reading one memory page (0x1000 bytes)
// at a time and looking for the signature in each page.
// We create a buffer sligthly larger than 0x1000 bytes in order to keep
// the very first bytes as the tail of the previous memory page.
// at a time and looking for the signature in each page. We create a buffer
// sligthly larger than 0x1000 bytes in order to accomodate the size of
// the memory page + the signature - 1. The very first bytes of the
// buffer are intended to be used as the tail of the previous memory page.
// This allows to scan across the memory page boundaries.

// We should use N - 1 but we resort to MEM_SIZE - 1 to avoid using [feature(generic_const_exprs)]
#[repr(packed)]
struct Buffer<const N: usize> {
_head: [u8; N],
_buffer: [u8; MEM_SIZE]
_head: [u8; N],
_buffer: [u8; MEM_SIZE - 1]
}

let buf =
let mut global_buffer =
// SAFETY: Using mem::zeroed is safe in this instance as the Buffer struct
// only contains u8, for which zeroed data represent a valid bit pattern.
unsafe {
let mut global_buffer = mem::zeroed::<Buffer<N>>();
slice::from_raw_parts_mut(&mut global_buffer as *mut _ as *mut u8, N + MEM_SIZE)
mem::zeroed::<Buffer<N>>()
};

let first = {
let buf =
// SAFETY: The buffer is guaranteed to be initialized due
// to using mem::zeroed() so we can safely return an u8 slice of it.
unsafe { slice::from_raw_parts_mut(buf.as_mut_ptr(), N) }
unsafe {
slice::from_raw_parts_mut(&mut global_buffer as *mut _ as *mut u8, size_of::<Buffer<N>>())
};

let last = {
let first =
// SAFETY: The buffer is guaranteed to be initialized due
// to using mem::zeroed() so we can safely return an u8 slice of it.
unsafe { slice::from_raw_parts_mut(buf.as_mut_ptr().add(MEM_SIZE), N) }
unsafe {
slice::from_raw_parts_mut(buf.as_mut_ptr(), N - 1)
};

let last =
// SAFETY: The buffer is guaranteed to be initialized due
// to using mem::zeroed() so we can safely return an u8 slice of it.
unsafe {
slice::from_raw_parts_mut(buf.as_mut_ptr().add(MEM_SIZE), N - 1)
};

let mut last_page_success = false;
while addr.value() < overall_end {
// We round up to the 4 KiB address boundary as that's a single
// page, which is safe to read either fully or not at all. We do
// this to reduce the number of syscalls as much as possible, as the
// syscall overhead is quite high.
let end = (addr.value() & !((4 << 10) - 1)) + (4 << 10).min(overall_end);
let len = end - addr.value();
let end = ((addr.value() & !((4 << 10) - 1)) + (4 << 10)).min(overall_end);
let len = end.saturating_sub(addr.value()) as usize;

// If we read the previous memory page successfully, then we can copy the last
// elements to the start of the buffer. Otherwise, we just fill it with zeroes.
if last_page_success {
first.copy_from_slice(last);
last_page_success = false;
} else {
first.fill(0);
}

if process.read_into_buf(addr, &mut buf[N..][..len as usize]).is_ok() {
last_page_success = true;
if let Some(pos) = self.scan(&mut buf[..len as usize + N]) {
return Some(addr.add(pos as u64).add_signed(-(N as i64)));
if process.read_into_buf(addr, &mut buf[N - 1..][..len]).is_ok() {
// If the previous memory page has been read successfully, then we have copied
// the last N - 1 bytes to the start of the buf array, and we need to check
// starting from those in order to correctly identify possible signatures crossing
// memory page boundaries
if last_page_success {
if let Some(pos) = self.scan(&mut buf[..len + N - 1]) {
return Some(addr.add(pos as u64).add_signed(-(N as i64 - 1)));
}
// If the prevbious memory page wasn't read successfully, the first N - 1
// bytes are excluded from the signature verification
} else {

Check warning on line 269 in src/signature.rs

View workflow job for this annotation

GitHub Actions / Check clippy lints

this `else { if .. }` block can be collapsed
if let Some(pos) = self.scan(&mut buf[N - 1..][..len]) {
return Some(addr.add(pos as u64));
}
}

last_page_success = true;

};
addr = Address::new(end);
}
Expand Down

0 comments on commit 8af2c3e

Please sign in to comment.