netvl/acc_reader

A seekable wrapper for input streams in Rust

acc_reader's Issues

API Soundness issue in `fill_buf()` and `read_up_to()`

Hello fellow Rustacean,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

acc_reader/src/lib.rs

Lines 244 to 271 in 95a54aa

	impl<R: Read> BufRead for AccReader<R> {
	fn fill_buf(&mut self) -> io::Result<&[u8]> {
	let available = self.buf.len() - self.pos; // self.buf.len() >= pos
	if available == 0 {
	let old_len = self.buf.len();
	self.buf.reserve(self.inc);
	unsafe { self.buf.set_len(old_len + self.inc); }
	let (read, error) = match self.source.read(&mut self.buf[self.pos..]) {
	Ok(n) => (n, None),
	Err(e) => (0, Some(e)),
	};
	unsafe { self.buf.set_len(old_len + read); }
	if let Some(e) = error {
	Err(e)
	} else {
	Ok(&self.buf[self.pos..])
	}
	} else {
	Ok(&self.buf[self.pos..])
	}
	}
	fn consume(&mut self, amt: usize) {
	self.pos = cmp::min(self.pos + amt, self.buf.len());
	}
	}

acc_reader/src/lib.rs

Lines 190 to 219 in 95a54aa

	// Read from the stream into the internal buffer as much as possible,
	// but no more than the provided number of bytes.
	// Updates the buffer length to the actual number of bytes read, even
	// in case of errors.
	fn read_up_to(&mut self, n: u64) -> io::Result<()> {
	let old_len = self.buf.len();
	self.buf.reserve(n as usize);
	unsafe { self.buf.set_len(old_len + n as usize); }
	let mut error = None;
	let mut read = 0;
	{
	let mut target = &mut self.buf[old_len..];
	while !target.is_empty() {
	match self.source.read(target) {
	Ok(0) => break,
	Ok(n) => { read += n; let tmp = target; target = &mut tmp[n..]; }
	Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}
	Err(e) => { error = Some(e); break; },
	}
	}
	}
	unsafe { self.buf.set_len(old_len + read as usize); }
	if let Some(e) = error {
	Err(e)
	} else {
	Ok(())
	}
	}

fill_buf() and read_up_to() methods create an uninitialized buffer and pass it to user-provided Read implementation. This is unsound, because it allows safe Rust code to exhibit an undefined behavior (read from uninitialized memory).

This part from the Read trait documentation explains the issue:

It is your responsibility to make sure that buf is initialized before calling read. Calling read with an uninitialized buf (of the kind one obtains via MaybeUninit<T>) is not safe, and can lead to undefined behavior.