Skip to content

Commit

Permalink
Merge pull request #304 from Lucretiel/checked-access-len
Browse files Browse the repository at this point in the history
  • Loading branch information
@kornelski
kornelski authored Apr 19, 2022
2 parents aaba38c + 8fbc5b2 commit 36940d0
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 71 deletions.
123 changes: 53 additions & 70 deletions rmp-serde/src/decode.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
//! Generic MessagePack deserialization.
use std::convert::TryInto;
use std::error;
use std::fmt::{self, Display, Formatter};
use std::io::{self, Cursor, ErrorKind, Read};
use std::num::TryFromIntError;
use std::str::{self, Utf8Error};

use byteorder::{self, ReadBytesExt};
Expand Down Expand Up @@ -157,6 +159,13 @@ impl<'a> From<DecodeStringError<'a>> for Error {
}
}

impl From<TryFromIntError> for Error {
#[cold]
fn from(_: TryFromIntError) -> Self {
Error::OutOfRange
}
}

/// A Deserializer that reads bytes from a buffer.
///
/// # Note
Expand Down Expand Up @@ -327,8 +336,6 @@ impl<'de, R: ReadSlice<'de>, C: SerializerConfig> Deserializer<R, C> {
}

fn read_128(&mut self) -> Result<[u8; 16], Error> {
use std::convert::TryInto;

let marker = self.take_or_read_marker()?;

if marker != Marker::Bin8 {
Expand Down Expand Up @@ -508,35 +515,55 @@ impl<'de, 'a, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> f
Marker::FixStr(len) => Ok(len.into()),
Marker::Str8 => read_u8(&mut self.rd).map(u32::from),
Marker::Str16 => read_u16(&mut self.rd).map(u32::from),
Marker::Str32 | _ => read_u32(&mut self.rd).map(u32::from),
Marker::Str32 => read_u32(&mut self.rd).map(u32::from),
_ => unreachable!()
}?;
self.read_str_data(len, visitor)
}
Marker::FixArray(_) |
Marker::Array16 |
Marker::Array32 => {
let len = match marker {
Marker::FixArray(len) => Ok(len.into()),
Marker::Array16 => read_u16(&mut self.rd).map(u32::from),
Marker::Array32 | _ => read_u32(&mut self.rd).map(u32::from),
}?;
depth_count!(self.depth, visitor.visit_seq(SeqAccess::new(self, len as usize)))
Marker::FixArray(len) => len.into(),
Marker::Array16 => read_u16(&mut self.rd)?.into(),
Marker::Array32 => read_u32(&mut self.rd)?,
_ => unreachable!(),
};

depth_count!(self.depth, {
let mut seq = SeqAccess::new(self, len);
let res = visitor.visit_seq(&mut seq)?;
match seq.left {
0 => Ok(res),
excess => Err(Error::LengthMismatch(len - excess)),
}
})
}
Marker::FixMap(_) |
Marker::Map16 |
Marker::Map32 => {
let len = match marker {
Marker::FixMap(len) => Ok(len.into()),
Marker::Map16 => read_u16(&mut self.rd).map(u32::from),
Marker::Map32 | _ => read_u32(&mut self.rd).map(u32::from),
}?;
depth_count!(self.depth, visitor.visit_map(MapAccess::new(self, len as usize)))
Marker::FixMap(len) => len.into(),
Marker::Map16 => read_u16(&mut self.rd)?.into(),
Marker::Map32 => read_u32(&mut self.rd)?,
_ => unreachable!()
};

depth_count!(self.depth, {
let mut seq = MapAccess::new(self, len);
let res = visitor.visit_map(&mut seq)?;
match seq.left {
0 => Ok(res),
excess => Err(Error::LengthMismatch(len - excess)),
}
})
}
Marker::Bin8 | Marker::Bin16| Marker::Bin32 => {
Marker::Bin8 | Marker::Bin16 | Marker::Bin32 => {
let len = match marker {
Marker::Bin8 => read_u8(&mut self.rd).map(u32::from),
Marker::Bin16 => read_u16(&mut self.rd).map(u32::from),
Marker::Bin32 | _ => read_u32(&mut self.rd).map(u32::from),
Marker::Bin32 => read_u32(&mut self.rd).map(u32::from),
_ => unreachable!()
}?;
match read_bin_data(&mut self.rd, len)? {
Reference::Borrowed(buf) => visitor.visit_borrowed_bytes(buf),
Expand Down Expand Up @@ -652,66 +679,22 @@ impl<'de, 'a, R: ReadSlice<'de>, C: SerializerConfig> serde::Deserializer<'de> f
visitor.visit_u128(u128::from_be_bytes(buf))
}

fn deserialize_f32<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where V: Visitor<'de>
{
// This special case allows us to decode some integer types as floats when safe, and
// asked for.
//
// This allows interoperability with msgpack-lite, which performs an optimization of
// storing rounded floats as integers.
let f = match self.take_or_read_marker()? {
Marker::U8 => self.rd.read_data_u8().map(f32::from),
Marker::U16 => self.rd.read_data_u16().map(f32::from),
Marker::I8 => self.rd.read_data_i8().map(f32::from),
Marker::I16 => self.rd.read_data_i16().map(f32::from),
Marker::F32 => self.rd.read_data_f32(),
marker => {
self.marker = Some(marker);
return self.deserialize_any(visitor);
}
}?;
visitor.visit_f32(f)
}

fn deserialize_f64<V>(self, visitor: V) -> Result<V::Value, Self::Error>
where V: Visitor<'de>
{
// This special case allows us to decode some integer types as floats when safe, and
// asked for. This is here to be consistent with 'f32'.
let f = match self.take_or_read_marker()? {
Marker::U8 => self.rd.read_data_u8().map(f64::from),
Marker::U16 => self.rd.read_data_u16().map(f64::from),
Marker::U32 => self.rd.read_data_u32().map(f64::from),
Marker::I8 => self.rd.read_data_i8().map(f64::from),
Marker::I16 => self.rd.read_data_i16().map(f64::from),
Marker::I32 => self.rd.read_data_i32().map(f64::from),
Marker::F32 => self.rd.read_data_f32().map(f64::from),
Marker::F64 => self.rd.read_data_f64(),
marker => {
self.marker = Some(marker);
return self.deserialize_any(visitor);
}
}?;
visitor.visit_f64(f)
}

forward_to_deserialize_any! {
bool u8 u16 u32 u64 i8 i16 i32 i64 char
str string bytes byte_buf unit seq map
struct identifier tuple tuple_struct
ignored_any
bool u8 u16 u32 u64 i8 i16 i32 i64 f32
f64 char str string bytes byte_buf unit
seq map struct identifier tuple
tuple_struct ignored_any
}
}

struct SeqAccess<'a, R, C> {
de: &'a mut Deserializer<R, C>,
left: usize,
left: u32,
}

impl<'a, R: 'a, C> SeqAccess<'a, R, C> {
#[inline]
fn new(de: &'a mut Deserializer<R, C>, len: usize) -> Self {
fn new(de: &'a mut Deserializer<R, C>, len: u32) -> Self {
SeqAccess {
de,
left: len,
Expand All @@ -736,17 +719,17 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::SeqAccess<'de> fo

#[inline(always)]
fn size_hint(&self) -> Option<usize> {
Some(self.left)
self.left.try_into().ok()
}
}

struct MapAccess<'a, R, C> {
de: &'a mut Deserializer<R, C>,
left: usize,
left: u32,
}

impl<'a, R: 'a, C> MapAccess<'a, R, C> {
fn new(de: &'a mut Deserializer<R, C>, len: usize) -> Self {
fn new(de: &'a mut Deserializer<R, C>, len: u32) -> Self {
MapAccess {
de,
left: len,
Expand All @@ -763,7 +746,7 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::MapAccess<'de> fo
{
if self.left > 0 {
self.left -= 1;
Ok(Some(seed.deserialize(&mut *self.de)?))
seed.deserialize(&mut *self.de).map(Some)
} else {
Ok(None)
}
Expand All @@ -778,7 +761,7 @@ impl<'de, 'a, R: ReadSlice<'de> + 'a, C: SerializerConfig> de::MapAccess<'de> fo

#[inline(always)]
fn size_hint(&self) -> Option<usize> {
Some(self.left)
self.left.try_into().ok()
}
}

Expand Down
2 changes: 1 addition & 1 deletion rmp-serde/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl<'a> RawRef<'a> {
pub fn as_bytes(&self) -> &[u8] {
match self.s {
Ok(s) => s.as_bytes(),
Err(ref err) => err.0,
Err((bytes, _err)) => bytes,
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions rmp-serde/tests/round.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,50 @@ fn roundtrip_some() {
assert_roundtrips(Some("hi".to_string()));
}

/// Some types don't fully consume their input SeqAccess, leading to incorrect
/// deserializes.
///
/// https://github.com/3Hren/msgpack-rust/issues/287
#[test]
fn checked_seq_access_len()
{
#[derive(Serialize)]
struct Input {
a: [&'static str; 4],
d: &'static str,
}

#[allow(dead_code)]
#[derive(Deserialize, Debug)]
struct Output {
a: [String; 2],
c: String,
}

let mut buffer = Vec::new();
let mut serializer = Serializer::new(&mut buffer)
.with_binary()
.with_struct_map();

// The bug is basically that Output will successfully deserialize from input
// because the [String; 0] deserializer doesn't drain the SeqAccess, and
// the two fields it leaves behind can then be deserialized into `v`

let data = Input {
a: ["b", "b", "c", "c"],
d: "d",
};

data.serialize(&mut serializer).expect("failed to serialize");

let mut deserializer = rmp_serde::Deserializer::new(
Cursor::new(&buffer)
).with_binary();

Output::deserialize(&mut deserializer)
.expect_err("Input round tripped into Output; this shouldn't happen");
}

#[ignore]
#[test]
fn roundtrip_some_failures() {
Expand Down

0 comments on commit 36940d0

Please sign in to comment.