Do not access uninitialized memory in CodedInputStream

protobuf/src/buf_read_iter.rs

@@ -3,6 +3,7 @@ use std::io::BufRead;
 use std::io::BufReader;
 use std::io::Read;
 use std::mem;
+use std::mem::MaybeUninit;
 use std::u64;
 
 #[cfg(feature = "bytes")]
@@ -17,6 +18,8 @@ use bytes::BytesMut;
 use crate::buf_read_or_reader::BufReadOrReader;
 use crate::coded_input_stream::READ_RAW_BYTES_MAX_ALLOC;
 use crate::error::WireError;
+use crate::misc::maybe_uninit_write_slice;
+use crate::misc::vec_spare_capacity_mut;
 use crate::ProtobufError;
 use crate::ProtobufResult;
 
@@ -288,7 +291,7 @@ impl<'ignore> BufReadIter<'ignore> {
         Ok(len)
     }
 
-    fn read_exact_slow(&mut self, buf: &mut [u8]) -> ProtobufResult<()> {
+    fn read_exact_slow(&mut self, buf: &mut [MaybeUninit<u8>]) -> ProtobufResult<()> {
         if self.bytes_until_limit() < buf.len() as u64 {
             return Err(ProtobufError::WireError(WireError::UnexpectedEof));
         }
@@ -302,7 +305,7 @@ impl<'ignore> BufReadIter<'ignore> {
         match self.input_source {
             InputSource::Read(ref mut buf_read) => {
                 buf_read.consume(consume);
-                buf_read.read_exact(buf)?;
+                buf_read.read_exact_uninit(buf)?;
             }
             _ => {
                 return Err(ProtobufError::WireError(WireError::UnexpectedEof));
@@ -317,10 +320,13 @@ impl<'ignore> BufReadIter<'ignore> {
     }
 
     #[inline]
-    pub(crate) fn read_exact(&mut self, buf: &mut [u8]) -> ProtobufResult<()> {
+    pub(crate) fn read_exact(&mut self, buf: &mut [MaybeUninit<u8>]) -> ProtobufResult<()> {
         if self.remaining_in_buf_len() >= buf.len() {
             let buf_len = buf.len();
-            buf.copy_from_slice(&self.buf[self.pos_within_buf..self.pos_within_buf + buf_len]);
+            maybe_uninit_write_slice(
+                buf,
+                &self.buf[self.pos_within_buf..self.pos_within_buf + buf_len],
+            );
             self.pos_within_buf += buf_len;
             return Ok(());
         }
@@ -362,7 +368,7 @@ impl<'ignore> BufReadIter<'ignore> {
             target.reserve_exact(count);
 
             unsafe {
-                self.read_exact(&mut target.get_unchecked_mut(..count))?;
+                self.read_exact(&mut vec_spare_capacity_mut(target)[..count])?;
                 target.set_len(count);
             }
         }
@@ -505,7 +511,12 @@ mod test {
         let _prev_limit = buf_read_iter.push_limit(5);
         buf_read_iter.read_byte().expect("read_byte");
         buf_read_iter
-            .read_exact(&mut [1, 2, 3, 4])
+            .read_exact(&mut [
+                MaybeUninit::uninit(),
+                MaybeUninit::uninit(),
+                MaybeUninit::uninit(),
+                MaybeUninit::uninit(),
+            ])
             .expect("read_exact");
         assert!(buf_read_iter.eof().expect("eof"));
     }

protobuf/src/buf_read_or_reader.rs

@@ -40,7 +40,7 @@ impl<'a> Read for BufReadOrReader<'a> {
 
 impl<'a> BufReadOrReader<'a> {
     /// Similar to `read_exact` but reads into `MaybeUninit`.
-    pub(crate) fn _read_exact_uninit(
+    pub(crate) fn read_exact_uninit(
         &mut self,
         buf: &mut [MaybeUninit<u8>],
     ) -> Result<(), io::Error> {

protobuf/src/coded_input_stream.rs

@@ -6,6 +6,7 @@ use std::io;
 use std::io::BufRead;
 use std::io::Read;
 use std::mem;
+use std::mem::MaybeUninit;
 use std::slice;
 
 #[cfg(feature = "bytes")]
@@ -19,6 +20,7 @@ use crate::error::ProtobufError;
 use crate::error::ProtobufResult;
 use crate::error::WireError;
 use crate::message::Message;
+use crate::misc::maybe_ununit_array_assume_init;
 use crate::unknown::UnknownValue;
 use crate::wire_format;
 use crate::zigzag::decode_zig_zag_32;
@@ -104,13 +106,22 @@ impl<'a> CodedInputStream<'a> {
         self.source.bytes_until_limit()
     }
 
+    /// Read bytes into given `buf`.
+    #[inline]
+    fn read_exact_uninit(&mut self, buf: &mut [MaybeUninit<u8>]) -> ProtobufResult<()> {
+        self.source.read_exact(buf)
+    }
+
     /// Read bytes into given `buf`.
     ///
     /// Return `0` on EOF.
     // TODO: overload with `Read::read`
     pub fn read(&mut self, buf: &mut [u8]) -> ProtobufResult<()> {
-        self.source.read_exact(buf)?;
-        Ok(())
+        // SAFETY: same layout
+        let buf = unsafe {
+            slice::from_raw_parts_mut(buf.as_mut_ptr() as *mut MaybeUninit<u8>, buf.len())
+        };
+        self.read_exact_uninit(buf)
     }
 
     /// Read exact number of bytes as `Bytes` object.
@@ -248,24 +259,20 @@ impl<'a> CodedInputStream<'a> {
 
     /// Read little-endian 32-bit integer
     pub fn read_raw_little_endian32(&mut self) -> ProtobufResult<u32> {
-        let mut r = 0u32;
-        let bytes: &mut [u8] = unsafe {
-            let p: *mut u8 = mem::transmute(&mut r);
-            slice::from_raw_parts_mut(p, mem::size_of::<u32>())
-        };
-        self.read(bytes)?;
-        Ok(r.to_le())
+        let mut bytes = [MaybeUninit::uninit(); 4];
+        self.read_exact_uninit(&mut bytes)?;
+        // SAFETY: `read_exact` guarantees that the buffer is filled.
+        let bytes = unsafe { maybe_ununit_array_assume_init(bytes) };
+        Ok(u32::from_le_bytes(bytes))
     }
 
     /// Read little-endian 64-bit integer
     pub fn read_raw_little_endian64(&mut self) -> ProtobufResult<u64> {
-        let mut r = 0u64;
-        let bytes: &mut [u8] = unsafe {
-            let p: *mut u8 = mem::transmute(&mut r);
-            slice::from_raw_parts_mut(p, mem::size_of::<u64>())
-        };
-        self.read(bytes)?;
-        Ok(r.to_le())
+        let mut bytes = [MaybeUninit::uninit(); 8];
+        self.read_exact_uninit(&mut bytes)?;
+        // SAFETY: `read_exact` guarantees that the buffer is filled.
+        let bytes = unsafe { maybe_ununit_array_assume_init(bytes) };
+        Ok(u64::from_le_bytes(bytes))
     }
 
     /// Read tag
@@ -596,41 +603,7 @@ impl<'a> CodedInputStream<'a> {
     /// Read raw bytes into the supplied vector.  The vector will be resized as needed and
     /// overwritten.
     pub fn read_raw_bytes_into(&mut self, count: u32, target: &mut Vec<u8>) -> ProtobufResult<()> {
-        if false {
-            // Master uses this version, but keep existing version for a while
-            // to avoid possible breakages.
-            return self.source.read_exact_to_vec(count as usize, target);
-        }
-
-        let count = count as usize;
-
-        // TODO: also do some limits when reading from unlimited source
-        if count as u64 > self.source.bytes_until_limit() {
-            return Err(ProtobufError::WireError(WireError::TruncatedMessage));
-        }
-
-        unsafe {
-            target.set_len(0);
-        }
-
-        if count >= READ_RAW_BYTES_MAX_ALLOC {
-            // avoid calling `reserve` on buf with very large buffer: could be a malformed message
-
-            let mut take = self.by_ref().take(count as u64);
-            take.read_to_end(target)?;
-
-            if target.len() != count {
-                return Err(ProtobufError::WireError(WireError::TruncatedMessage));
-            }
-        } else {
-            target.reserve(count);
-            unsafe {
-                target.set_len(count);
-            }
-
-            self.source.read_exact(target)?;
-        }
-        Ok(())
+        self.source.read_exact_to_vec(count as usize, target)
     }
 
     /// Read exact number of bytes

protobuf/src/misc.rs

@@ -29,3 +29,16 @@ where
 
     unsafe { &mut *(this as *mut [MaybeUninit<T>] as *mut [T]) }
 }
+
+/// `MaybeUninit::array_assume_init` is not stable.
+#[inline]
+pub(crate) unsafe fn maybe_ununit_array_assume_init<T, const N: usize>(
+    array: [MaybeUninit<T>; N],
+) -> [T; N] {
+    // SAFETY:
+    // * The caller guarantees that all elements of the array are initialized
+    // * `MaybeUninit<T>` and T are guaranteed to have the same layout
+    // * `MaybeUninit` does not drop, so there are no double-frees
+    // And thus the conversion is safe
+    (&array as *const _ as *const [T; N]).read()
+}