Skip to content

Soundness: Exposure of uninitialized memory in IpDefragBuf #154

Description

@ia0

Note

This finding was identified during an agentic unsafe Rust code review orchestrated by @Manishearth and performed by Gemini AI.

In IpDefragBuf::add() (a safe function), the safety invariant of self.data is temporarily broken by self.data.set_len() but not always restored before returning. If the offset is greater than self.data.len() on entry, then any data in between is not initialized, and can be accessed with IpDefragBuf::data() (a safe function). The most simple fix is to use self.data.resize() instead of self.data.set_len(), but there are other options like initializing anything between the old length and the offset, or only exposing the longest initialized prefix through safe functions.

Minimal Reproduction (Miri)
use etherparse::IpFragOffset;
use etherparse::IpNumber;
use etherparse::defrag::IpDefragBuf;

fn main() {
    let mut buf = IpDefragBuf::new(IpNumber::UDP, Vec::new(), Vec::new());
    let offset = IpFragOffset::try_new(10).unwrap();
    let payload = [1, 2, 3, 4, 5, 6, 7, 8];
    buf.add(offset, true, &payload).unwrap();
    let _byte = buf.data()[0]; // UB: reading uninit at u8
}
% cargo miri run
error: Undefined Behavior: reading memory at alloc313[0x0..0x1], but memory is uninitialized at [0x0..0x1], and this operation requires initialized memory
  --> src/main.rs:10:17
   |
10 |     let _byte = buf.data()[0]; // UB: reading uninit at u8
   |                 ^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = 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
Original AI finding
  • Location: src/defrag/ip_defrag_buf.rs:121

  • Description: IpDefragBuf reconstructs fragmented IP packets inside an internal backing vector self.data: Vec<u8>. When an out-of-order packet fragment arrives (i.e., a fragment whose starting byte offset is greater than the current vector length), IpDefragBuf::add calculates the required_len up to the end of that fragment. If self.data.len() < required_len, it reserves capacity and executes:

    unsafe {
      self.data.set_len(required_len);
    }

    While the incoming payload is copied into self.data[offset..end], any intervening memory range between old_len and offset remains uninitialized heap memory returned by the global allocator.
    Although IpDefragBuf::is_complete() returns false until all gaps are filled, IpDefragBuf exposes safe public inspection APIs:

    • pub fn data(&self) -> &Vec<u8> (lines 48-50)
    • pub fn take_bufs(self) -> (Vec<u8>, Vec<IpFragRange>) (line 167)
    • Derived trait implementations for Clone, Debug, PartialEq, Eq, Hash.
  • Soundness Violation: According to the Rust Reference and std::vec::Vec::set_len safety documentation, all elements up to vector length must be initialized. Integers (u8) do not permit uninitialized bit patterns (MaybeUninit<u8>). Invoking set_len over uninitialized memory is immediate Undefined Behavior. Furthermore, allowing safe external code to read this uninitialized memory via data(), Debug, or Clone introduces information disclosure vulnerabilities, potentially leaking sensitive stale heap data from concurrent server operations.

  • Remediation: Remove unsafe { self.data.set_len(required_len); }. Instead, advance vector length safely by zero-initializing newly exposed memory:

    self.data.resize(required_len, 0);

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions