Skip to content

Initialise the internal buffer before handing it to the reader - #10

Open
icedracon wants to merge 1 commit into
andre-vm:masterfrom
icedracon:fix/uninitialized-buffer
Open

Initialise the internal buffer before handing it to the reader#10
icedracon wants to merge 1 commit into
andre-vm:masterfrom
icedracon:fix/uninitialized-buffer

Conversation

@icedracon

Copy link
Copy Markdown

Fixes #9.

with_capacity called set_len on a Vec that was never written to, and fill_buf passes that buffer straight to the wrapped Read impl. Read::read only recommends against reading buf rather than forbidding it, so an implementation that inspects its buffer observed uninitialised memory.

Miri on the current release:

error: Undefined Behavior: reading memory at alloc46971[0x0..0x1],
       but memory is uninitialized at [0x0..0x1],
       and this operation requires initialized memory
   = note: stack backtrace:
      0: <PeekingReader as std::io::Read>::read
      3: <RevBufReader<PeekingReader> as std::io::BufRead>::fill_buf
           at src/lib.rs:401:13

Change

Allocate a zeroed buffer instead:

let buffer = vec![0; capacity];

std::io::BufReader shipped exactly this for years before moving to BorrowedBuf/read_buf.

This also drops the inner.initializer().initialize(&mut buffer) call. That path sat behind feature = "read_initializer", which needs the #![feature(read_initializer)] nightly gate — a language feature that has since been removed from the compiler, so it could not be enabled on any current toolchain. The read_initializer feature flag itself and the Read::initializer impl are left alone; removing them is a separate cleanup if you want it.

Verification

  • new regression test test_buffer_handed_to_reader_is_initialized
  • existing suite passes (11 unit + 10 integration)
  • cargo +nightly miri test clean on the patched crate

Happy to adjust anything.

`with_capacity` called `set_len` on a `Vec` that was never written to, and
`fill_buf` passes that buffer straight to the wrapped `Read` impl. Since
`Read::read` only recommends against reading `buf` rather than forbidding
it, an implementation that inspects its buffer observed uninitialised
memory, which Miri reports as undefined behaviour.

Allocate a zeroed buffer instead. This also drops the `read_initializer`
call, whose nightly feature gate no longer exists in the compiler, so that
path could not be enabled on any current toolchain anyway.

Adds a regression test that fails under Miri if the buffer is left
uninitialised.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unsoundness: uninitialised buffer is passed to the wrapped Read implementation

1 participant