Potential Unsoundness in remove_from_payload_head and add_to_payload_head

[lwz23]

For both functions,

NetBricks/framework/src/interface/packet.rs

Line 332 in 71dfb94

pub fn remove_from_payload_head(&mut self, size: usize) -> Result<()> {

NetBricks/framework/src/interface/packet.rs

Line 344 in 71dfb94

pub fn add_to_payload_head(&mut self, size: usize) -> Result<()> {

pub fn remove_from_payload_head(&mut self, size: usize) -> Result<()> {
        unsafe {
            let src = self.data_base();
            let dst = src.offset(size as isize);
            ptr::copy_nonoverlapping(src, dst, size);
            (*self.mbuf).remove_data_beginning(size);
            Ok(())
        }
    }

    /// Add data to the head of the payload.
    #[inline]
    pub fn add_to_payload_head(&mut self, size: usize) -> Result<()> {
        unsafe {
            let added = (*self.mbuf).add_data_end(size);
            if added >= size {
                let src = self.payload();
                let dst = src.offset(size as isize);
                ptr::copy_nonoverlapping(src, dst, size);
                Ok(())
            } else {
                Err(ErrorKind::FailedAllocation.into())
            }
        }
    }

unsound behavior and undefined behavior (UB) can occur depending on how the size parameter is used and validated:

1. Overlapping Memory Regions in ptr::copy_nonoverlapping
   ptr::copy_nonoverlapping assumes that the src and dst memory regions do not overlap.
   If size is large enough that src and dst overlap, UB will occur.
2. Invalid or Out-of-Bounds Pointers
   If size causes src.offset(size as isize) (or equivalently, dst) to go out of bounds of the allocated memory, this will result in UB when accessing the memory.
   This could happen if:
   size exceeds the length of the payload or available data.
   The memory layout of self.data_base() or self.payload() does not accommodate the calculated offset.
3. Violating Rust's Borrowing Rules
   These functions modify memory using raw pointers (unsafe), which means aliasing rules could be violated.
   If src or dst overlaps with other active references, this would cause UB.
4. Misuse of (*self.mbuf)
   The behavior of (*self.mbuf).remove_data_beginning and (*self.mbuf).add_data_end is critical.
   If these methods do not properly validate size or leave the memory in an inconsistent state, subsequent pointer operations may cause UB.
5. Lack of Bounds Checks
   There are no checks to ensure size is valid before performing pointer arithmetic or calling functions like offset or copy_nonoverlapping.
   If size is too large (e.g., user passes usize::MAX), pointer arithmetic could result in integer overflow or invalid memory access.

Can Users Trigger UB by Controlling size?
Yes, users can trigger UB by manipulating the size parameter in several ways:

1. Pass an Excessively Large size:
   If size is larger than the available payload or exceeds the memory bounds of self.data_base() or self.payload(), pointer arithmetic (e.g., src.offset(size as isize)) will result in invalid pointers and UB.
2. Cause Overlapping Memory Regions:
   If size causes src and dst to overlap, ptr::copy_nonoverlapping will trigger UB. This can happen if the layout of the payload does not correctly account for size.
3. Exploit Lack of Internal Checks:
   If the internal methods remove_data_beginning or add_data_end do not properly verify or update memory state based on size, subsequent operations could break invariants and lead to UB.