UB found by Miri

[yilin0518]

Hi!

We are a team of researchers studying memory safety in Rust. As part of our ongoing research, we tested circular (version: 0.3.0) and found that the following code snippet is reported as undefined behavior by Miri:

The generated case is:

#![feature(allocator_api)]
extern crate alloc;
use circular::*;
fn main() {
    let v1 = [61u8, 15u8, 154u8];
    let v2 = Box::new(v1);
    let v3 = &v2[..];
    let mut v4 = Buffer::from_slice(v3);
    let v15: &'_ Buffer = &v4;
    let v16 = Buffer::available_space(v15);
    let v22: &'_ mut Buffer = &mut v4;
    let v23 = Buffer::consume_noshift(v22, v16);
    let v26: &'_ mut Buffer = &mut v4;
    let v27 = Buffer::fill(v26, v23);
    let v34: &'_ mut Buffer = &mut v4;
    let v35 = [90u8, 118u8, 17u8];
    let v36 = Box::new(v35);
    let v37 = &v36[..];
    let v38 = Buffer::replace_slice(v34, v37, v16, v27);
}

Minimal Problematic Snippet

use circular::Buffer;

fn main() {
    let mut buf = Buffer::from_slice(&[1u8, 2u8, 3u8]);
    buf.replace_slice(&[4u8, 5u8, 6u8], 0, 0);
}

Miri report

error: Undefined Behavior: memory access failed: attempting to access 3 bytes, but got alloc226+0x3 which is at or beyond the end of the allocation of size 3 bytes
   --> /home/rose/projects/lifesonar-tests1/new_crates/analyze-poc/circular-0.3.0/src/lib.rs:266:9
    |
266 | ...   ptr::copy((&self.memory[start+length..self.end]).as_ptr(), (&mut self.memory[start+data_len..]).as_mut_ptr(), self.end - (start + length));
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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
help: alloc226 was allocated here:
   --> src/main.rs:6:19
    |
  6 |     let mut buf = Buffer::with_capacity(3);
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `circular::Buffer::replace_slice` at /home/rose/projects/lifesonar-tests1/new_crates/analyze-poc/circular-0.3.0/src/lib.rs:266:9: 266:147
note: inside `main`
   --> src/main.rs:8:5
    |
  8 |     buf.replace_slice(&[4, 5, 6], 0, 0);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

Additional Triggering API Combinations

The same UB path is reached via different safe API sequences that leave the buffer full (position == 0, end == capacity) and then call replace_slice with length = 0:

use circular::Buffer;
use std::io::Write;

fn main() {
    // Variant 1: fill via Write::write, then replace
    let mut buf = Buffer::with_capacity(3);
    buf.write_all(&[1, 2, 3]).unwrap();
    buf.replace_slice(&[4, 5, 6], 0, 0);
}

use circular::Buffer;
use std::io::{Read, Write};

fn main() {
    // Variant 2: consume + fill cycle, then replace on a full buffer
    let mut buf = Buffer::from_slice(&[1u8, 2u8, 3u8]);
    let space = buf.available_space();       // 0
    buf.consume_noshift(space);              // position stays 0
    buf.fill(space);                         // end stays 3
    buf.replace_slice(&[4, 5, 6], 0, 0);
}

Root-Cause Hypothesis

After analyzing the Miri report and the source code, we believe the UB is rooted in two related bugs in replace_slice (src/lib.rs:247–272):

1. Insufficient guard condition (line 249): The first guard checks start + length > self.available_data(). When length = 0 and start = 0, this evaluates to 0 > available_data, which is false — so the guard does not reject the call. However, the growth branch then constructs self.memory[start + data_len ..] which, when the buffer is full (end == capacity), starts at or beyond the end of the allocation, causing an out-of-bounds read.

2. Incorrect pointer arithmetic in the growth branch (line 266): The code correctly computes begin = self.position + start, but the ptr::copy on line 266 uses start instead of begin for both the source and destination slices. When position > 0, this causes ptr::copy to operate on wrong memory offsets, leading to out-of-bounds access.

The same issue exists in the shrink branch (line 261), which also uses start instead of begin.

Suggested Fix

pub fn replace_slice(&mut self, data: &[u8], start: usize, length: usize) -> Option<usize> {
    let data_len = data.len();
    let begin    = self.position + start;

    // Fix 1: guard on begin instead of raw start
    if start + length > self.available_data() ||
       begin + data_len > self.capacity {
      return None
    }

    unsafe {
      let slice_end = begin + data_len;

      if data_len < length {
        ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
        // Fix 2 (shrink branch): use begin instead of start
        ptr::copy((&self.memory[begin+length..self.end]).as_ptr(),
                  (&mut self.memory[slice_end..]).as_mut_ptr(),
                  self.end - (begin + length));
        self.end = self.end - (length - data_len);
      } else {
        // Fix 3 (growth branch): use begin instead of start
        ptr::copy((&self.memory[begin+length..self.end]).as_ptr(),
                  (&mut self.memory[begin+data_len..]).as_mut_ptr(),
                  self.end - (begin + length));
        ptr::copy(data.as_ptr(), (&mut self.memory[begin..slice_end]).as_mut_ptr(), data_len);
        self.end = self.end + data_len - length;
      }
    }
    Some(self.available_data())
}

We would appreciate it if you could take a look and confirm whether this behavior indicates a real issue, or if it is a false positive / expected limitation of Miri.

Thank you very much for your time and for maintaining this great project!