Unsoundness in raw_cksum

[lwz23]

Hello, thank you for your contribution in this project, I an testing our static analysis tool in github's Rust project and I notice the following code:

kytan/src/packet.rs

Line 62 in f54b2f8

fn raw_cksum<T>(buf: *const T, len: usize) -> u16 {

fn raw_cksum<T>(buf: *const T, len: usize) -> u16 {
    let mut sum = Wrapping(0);
    let mut remaining_len = len;
    let mut ptr = buf as *const u16;
    while remaining_len >= 2 {
        unsafe {
            sum += Wrapping(*ptr);
            ptr = ptr.offset(1);
        }
        remaining_len -= 2;
    }
    if remaining_len == 1 {
        unsafe {
            sum += Wrapping(*(ptr as *const u8) as u16);
        }
    }
    sum.0
}

The issue is in udptcp_cksum where it calculates l4_len based on the IP header's total_length field without validating that this matches the actual size of the provided l4 object:
let l4_len = (u16::from_be(ip.total_length) as usize) - mem::size_of::<Ipv4Header>();
Then raw_cksum uses this potentially incorrect length to read memory:

while remaining_len >= 2 {
    unsafe {
        sum += Wrapping(*ptr);
        ptr = ptr.offset(1);
    }
    remaining_len -= 2;
}

If total_length is manipulated to be larger than the actual size of the l4 object, this will read beyond the bounds of the object, causing undefined behavior. Although it is a private function, I notice a possible way to call this function from a pub function udptcp_cksum.

pub fn udptcp_cksum -> fn raw_cksum

Poc

fn main() {
    // Create an IPv4 header with an artificially large total_length field
    let mut ip_header = Ipv4Header {
        version_ihl: 0x45,  // Version 4, IHL 5 (20 bytes)
        total_length: u16::to_be(1000), // Claim packet is 1000 bytes
        // Initialize other required fields with valid values
        protocol: 6, // TCP
        source_address: [192, 168, 1, 1],
        destination_address: [192, 168, 1, 2],
        // Other fields can be zero
        type_of_service: 0,
        identification: 0,
        flags_fragment_offset: 0,
        time_to_live: 64,
        header_checksum: 0,
    };

    // Create a small TCP header - much smaller than what total_length claims
    let small_tcp_header = [0u8; 20]; // Only 20 bytes

    // This will cause undefined behavior in raw_cksum when it tries to read
    // beyond the bounds of small_tcp_header based on the inflated total_length
    let checksum = udptcp_cksum(&ip_header, &small_tcp_header);
}

[lwz23]

Here is my Poc

use std::{mem, num::Wrapping};


pub struct Ipv4Header {
    pub version_ihl: u8,            // IP version (= 4) + Internet header length
    pub type_of_service: u8,        // Type of service
    pub total_length: u16,          // Total length in octets
    pub identification: u16,        // Identification
    pub flags_fragment_offset: u16, // 3-bits Flags + Fragment Offset
    pub time_to_live: u8,           // Time To Live
    pub protocol: u8,               // Protocol
    pub header_checksum: u16,       // Checksum
    pub source_address: u32,        // Source Address
    pub destination_address: u32,   // Destination Address
}

pub fn udptcp_cksum<T>(ip: &Ipv4Header, l4: &T) {
    let l4_len = (u16::from_be(ip.total_length) as usize) - mem::size_of::<Ipv4Header>();
    let mut cksum = raw_cksum(l4 as *const T, l4_len) as u32;
}

fn raw_cksum<T>(buf: *const T, len: usize) -> u16 {
    let mut sum = Wrapping(0);
    let mut remaining_len = len;
    let mut ptr = buf as *const u16;
    while remaining_len >= 2 {
        unsafe {
            sum += Wrapping(*ptr);
            ptr = ptr.offset(1);
        }
        remaining_len -= 2;
    }
    if remaining_len == 1 {
        unsafe {
            sum += Wrapping(*(ptr as *const u8) as u16);
        }
    }
    sum.0
}

fn main() {
    // Create an IPv4 header with an artificially large total_length field
    let mut ip_header = Ipv4Header {
        version_ihl: 0x45,  // Version 4, IHL 5 (20 bytes)
        total_length: u16::to_be(1000), // Claim packet is 1000 bytes
        // Initialize other required fields with valid values
        protocol: 6, // TCP
        source_address: u32::from_be_bytes([192, 168, 1, 1]),
        destination_address: u32::from_be_bytes([192, 168, 1, 2]),
        // Other fields can be zero
        type_of_service: 0,
        identification: 0,
        flags_fragment_offset: 0,
        time_to_live: 64,
        header_checksum: 0,
    };

    // Create a small TCP header - much smaller than what total_length claims
    let small_tcp_header = [0u8; 20]; // Only 20 bytes

    // This will cause undefined behavior in raw_cksum when it tries to read
    // beyond the bounds of small_tcp_header based on the inflated total_length
    let checksum = udptcp_cksum(&ip_header, &small_tcp_header);
}

Rrsult

error: Undefined Behavior: memory access failed: expected a pointer to 2 bytes of memory, but got alloc481+0x14 which is at or beyond the end of the allocation of size 20 bytes
  --> src\main.rs:28:29
   |
28 |             sum += Wrapping(*ptr);
   |                             ^^^^ memory access failed: expected a pointer to 2 bytes of memory, but got alloc481+0x14 which is at or beyond the end of the allocation of size 20 bytes
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information