From c06b18abed3646d388206f85c3ecd700e121f8b4 Mon Sep 17 00:00:00 2001 From: Josh Mcguigan Date: Wed, 30 Oct 2019 05:18:08 -0700 Subject: [PATCH] improve safety of fill_buffer - see issue #260 --- src/de/read.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/de/read.rs b/src/de/read.rs index f18f789..ffc5ae2 100644 --- a/src/de/read.rs +++ b/src/de/read.rs @@ -1,6 +1,6 @@ use error::Result; use serde; -use std::io; +use std::{io, slice}; /// An optional Read trait for advanced Bincode usage. /// @@ -136,16 +136,32 @@ where R: io::Read, { fn fill_buffer(&mut self, length: usize) -> Result<()> { + // We first reserve the space needed in our buffer. let current_length = self.temp_buffer.len(); if length > current_length { self.temp_buffer.reserve_exact(length - current_length); } + // Then create a slice with the length as our desired length. This is + // safe as long as we only write (no reads) to this buffer, because + // `reserve_exact` above has allocated this space. + let buf = unsafe { + slice::from_raw_parts_mut(self.temp_buffer.as_mut_ptr(), length) + }; + + // This method is assumed to properly handle slices which include + // uninitialized bytes (as ours does). See discussion at the link below. + // https://github.com/servo/bincode/issues/260 + self.reader.read_exact(buf)?; + + // Only after `read_exact` successfully returns do we set the buffer + // length. By doing this after the call to `read_exact`, we can avoid + // exposing uninitialized memory in the case of `read_exact` returning + // an error. unsafe { self.temp_buffer.set_len(length); } - self.reader.read_exact(&mut self.temp_buffer)?; Ok(()) } }