Fix unchecked vector pre-allocation

Fixes 3Hren#151.
dbrgn · Oct 3, 2019 · f6d4671 · f6d4671
1 parent d6c6c67
commit f6d4671
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 8 deletions.
diff --git a/rmpv/src/decode/value.rs b/rmpv/src/decode/value.rs
@@ -1,4 +1,4 @@
-use std::io::Read;
+use std::io::{self, Read};
 
 use rmp::Marker;
 use rmp::decode::{read_marker, read_data_u8, read_data_u16, read_data_u32, read_data_u64,
@@ -9,7 +9,9 @@ use crate::{Utf8String, Value};
 use super::Error;
 
 fn read_array_data<R: Read>(rd: &mut R, mut len: usize) -> Result<Vec<Value>, Error> {
-    let mut vec = Vec::with_capacity(len);
+    // Note: Do not preallocate a Vec of size `len`.
+    // See https://github.com/3Hren/msgpack-rust/issues/151
+    let mut vec = Vec::new();
 
     while len > 0 {
         vec.push(read_value(rd)?);
@@ -20,7 +22,9 @@ fn read_array_data<R: Read>(rd: &mut R, mut len: usize) -> Result<Vec<Value>, Er
 }
 
 fn read_map_data<R: Read>(rd: &mut R, mut len: usize) -> Result<Vec<(Value, Value)>, Error> {
-    let mut vec = Vec::with_capacity(len);
+    // Note: Do not preallocate a Vec of size `len`.
+    // See https://github.com/3Hren/msgpack-rust/issues/151
+    let mut vec = Vec::new();
 
     while len > 0 {
         vec.push((read_value(rd)?, read_value(rd)?));
@@ -44,9 +48,16 @@ fn read_str_data<R: Read>(rd: &mut R, len: usize) -> Result<Utf8String, Error> {
 }
 
 fn read_bin_data<R: Read>(rd: &mut R, len: usize) -> Result<Vec<u8>, Error> {
-    let mut buf = Vec::with_capacity(len);
-    buf.resize(len as usize, 0u8);
-    rd.read_exact(&mut buf[..]).map_err(Error::InvalidDataRead)?;
+    // Note: Do not preallocate a Vec of size `len`.
+    // See https://github.com/3Hren/msgpack-rust/issues/151
+    let mut buf = Vec::new();
+    let bytes_read = rd.take(len as u64).read_to_end(&mut buf).map_err(Error::InvalidDataRead)?;
+    if bytes_read != len {
+        return Err(Error::InvalidDataRead(io::Error::new(
+            io::ErrorKind::UnexpectedEof,
+            format!("Expected {} bytes, read {} bytes", len, bytes_read),
+        )));
+    }
 
     Ok(buf)
 }

diff --git a/rmpv/src/decode/value_ref.rs b/rmpv/src/decode/value_ref.rs
@@ -53,7 +53,9 @@ fn read_ext_body<'a, R>(rd: &mut R, len: usize) -> Result<(i8, &'a [u8]), Error>
 fn read_array_data<'a, R>(rd: &mut R, mut len: usize) -> Result<Vec<ValueRef<'a>>, Error>
     where R: BorrowRead<'a>
 {
-    let mut vec = Vec::with_capacity(len);
+    // Note: Do not preallocate a Vec of size `len`.
+    // See https://github.com/3Hren/msgpack-rust/issues/151
+    let mut vec = Vec::new();
 
     while len > 0 {
         vec.push(read_value_ref(rd)?);
@@ -66,7 +68,9 @@ fn read_array_data<'a, R>(rd: &mut R, mut len: usize) -> Result<Vec<ValueRef<'a>
 fn read_map_data<'a, R>(rd: &mut R, mut len: usize) -> Result<Vec<(ValueRef<'a>, ValueRef<'a>)>, Error>
     where R: BorrowRead<'a>
 {
-    let mut vec = Vec::with_capacity(len);
+    // Note: Do not preallocate a Vec of size `len`.
+    // See https://github.com/3Hren/msgpack-rust/issues/151
+    let mut vec = Vec::new();
 
     while len > 0 {
         vec.push((read_value_ref(rd)?, read_value_ref(rd)?));

diff --git a/rmpv/tests/decode.rs b/rmpv/tests/decode.rs
@@ -361,3 +361,39 @@ fn from_array_of_two_integers() {
     let vec = vec![Value::from(4), Value::from(42)];
     assert_eq!(Value::Array(vec), read_value(&mut &buf[..]).unwrap());
 }
+
+#[test]
+fn invalid_buf_size_bin32() {
+    // This invalid buffer requests a 4 GiB byte vec.
+    let buf: &[u8] = &[0xc6, 0xff, 0xff, 0xff, 0xff, 0x00];
+    match read_value(&mut &buf[..]) {
+        Ok(_) => panic!("Unexpected success"),
+        Err(Error::InvalidDataRead(_)) => { /* expected */ },
+        Err(e) => panic!("Unexpected error: {}", e),
+    }
+}
+
+#[test]
+fn invalid_buf_size_arr() {
+    // This invalid buffer requests a nested array of depth 10.
+    // All arrays contain the maximum possible number of elements.
+    // If a byte is preallocated for every array content,
+    // that would require 40 GiB of RAM.
+    let buf: &[u8] = &[
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+        0xdd, 0xff, 0xff, 0xff, 0xff,
+    ];
+    match read_value(&mut &buf[..]) {
+        Ok(_) => panic!("Unexpected success"),
+        Err(Error::InvalidMarkerRead(_)) => { /* expected */ },
+        Err(e) => panic!("Unexpected error: {}", e),
+    }
+}