IoReader::read(): user-provided Read on uninitialized buffer may cause UB

[JOE1994]

Hello 🦀,
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

ms3d/src/read.rs

Lines 18 to 27 in 9bba59d

	impl<R: io::Read> Read for IoReader<R> {
	fn read(&mut self, len: usize) -> io::Result<&[u8]> {
	unsafe {
	self.buf.reserve(len);
	let slice = self.buf.get_unchecked_mut(..len);
	self.rdr.read_exact(slice)?;
	Ok(slice)
	}
	}
	}

IoReader::read() method creates an uninitialized buffer and passes 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 method is invokable from Model::from_reader() method.

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.

How to fix the issue?

The Naive & safe way to fix the issue is to always zero-initialize a buffer before lending it to a user-provided Read implementation. Note that this approach will add runtime performance overhead of zero-initializing the buffer.

As of Jan 2021, there is not yet an ideal fix that works in stable Rust with no performance overhead. Below are links to relevant discussions & suggestions for the fix.

• Well-written document regarding the issue
• Rust RFC 2930
• nightly feature std::io::Initializer
• Discussion in Rust Internals Forum

[JOE1994]

Thank you for the quick fix 👍