Heap buffer overflow in read_to_end_with_reservation()

[Qwaz]

rust/library/std/src/io/mod.rs

Lines 358 to 403 in c97f11a

	fn read_to_end_with_reservation<R, F>(
	r: &mut R,
	buf: &mut Vec<u8>,
	mut reservation_size: F,
	) -> Result<usize>
	where
	R: Read + ?Sized,
	F: FnMut(&R) -> usize,
	{
	let start_len = buf.len();
	let mut g = Guard { len: buf.len(), buf };
	let ret;
	loop {
	if g.len == g.buf.len() {
	unsafe {
	// FIXME(danielhenrymantilla): #42788
	//
	// - This creates a (mut) reference to a slice of
	// _uninitialized_ integers, which is **undefined behavior**
	//
	// - Only the standard library gets to soundly "ignore" this,
	// based on its privileged knowledge of unstable rustc
	// internals;
	g.buf.reserve(reservation_size(r));
	let capacity = g.buf.capacity();
	g.buf.set_len(capacity);
	r.initializer().initialize(&mut g.buf[g.len..]);
	}
	}
	match r.read(&mut g.buf[g.len..]) {
	Ok(0) => {
	ret = Ok(g.len - start_len);
	break;
	}
	Ok(n) => g.len += n,
	Err(ref e) if e.kind() == ErrorKind::Interrupted => {}
	Err(e) => {
	ret = Err(e);
	break;
	}
	}
	}
	ret
	}

At line 393, the guard object's .len field is incremented by the value returned from a read implementation. If a questionable Read returns a value larger than the buffer size, it will take that value and set the length of the vector over the boundary.

This bug is reachable from Read::read_to_end() and Read::read_to_string().

Here is a playground link that demonstrates the bug. It segfaults with double free or corruption (out).

[sfackler]

This is definitely broken - nice find.

A simple fix would be to just add an assert!(self.len <= self.buf.len()); in the drop impl:

rust/library/std/src/io/mod.rs

Lines 302 to 306 in c97f11a

	fn drop(&mut self) {
	unsafe {
	self.buf.set_len(self.len);
	}
	}

You can still get weird behavior in read_to_end_with_reservation, but it should be memory safe and it seems best to avoid a bunch of extra logic in the main loop.

[sfackler]

It looks like the bug was introduced in ecbb896.

[JohnTitor]

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.