From d2c100fd3e11983c0018b7061a4e2d68e841cd5a Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Fri, 3 Jul 2026 12:27:35 +0800 Subject: [PATCH] fix(filemeta): guard metacache decoding against corrupt length prefixes (#4226) --- crates/filemeta/src/filemeta.rs | 56 ++++++++++- crates/filemeta/src/filemeta/codec.rs | 37 ++++++- crates/filemeta/src/filemeta/msgp_decode.rs | 78 ++++++++++++++- crates/filemeta/src/filemeta/version.rs | 87 +++++++---------- crates/filemeta/src/filemeta_inline.rs | 68 +++++++------ crates/filemeta/src/metacache.rs | 102 +++++++++++++++++--- 6 files changed, 333 insertions(+), 95 deletions(-) diff --git a/crates/filemeta/src/filemeta.rs b/crates/filemeta/src/filemeta.rs index 79a83b015..27bded18c 100644 --- a/crates/filemeta/src/filemeta.rs +++ b/crates/filemeta/src/filemeta.rs @@ -99,7 +99,7 @@ pub fn is_skip_meta_key(key: &str) -> bool { mod codec; mod inline_data; -mod msgp_decode; +pub(crate) mod msgp_decode; mod validation; mod version; @@ -919,6 +919,60 @@ mod test { use proptest::collection::vec; use proptest::prelude::*; + /// Wraps a raw meta block in a valid XL2 container (header, bin32 length + /// prefix, and CRC trailer) so decode tests exercise the meta parsing + /// itself rather than the envelope checks. + fn build_xl_buffer(meta: &[u8]) -> Vec { + let mut buf = Vec::new(); + buf.extend_from_slice(&XL_FILE_HEADER); + buf.extend_from_slice(&XL_FILE_VERSION_MAJOR.to_le_bytes()); + buf.extend_from_slice(&XL_FILE_VERSION_MINOR.to_le_bytes()); + buf.push(0xc6); // bin32 + buf.extend_from_slice(&(meta.len() as u32).to_be_bytes()); + buf.extend_from_slice(meta); + let crc = xxh64::xxh64(meta, XXHASH_SEED) as u32; + buf.push(0xce); // u32 + buf.extend_from_slice(&crc.to_be_bytes()); + buf + } + + /// Regression test for rustfs/rustfs#2715: a corrupted version count in + /// xl.meta must yield a decode error instead of sizing a huge allocation + /// from the bogus count (which aborts the whole process). + #[test] + fn test_unmarshal_rejects_absurd_version_count() { + let mut meta = Vec::new(); + rmp::encode::write_uint(&mut meta, XL_HEADER_VERSION as u64).unwrap(); + rmp::encode::write_uint(&mut meta, XL_META_VERSION as u64).unwrap(); + // Claim ~10^15 versions with no version data behind it. + rmp::encode::write_sint(&mut meta, 1i64 << 50).unwrap(); + + let buf = build_xl_buffer(&meta); + let mut fm = FileMeta::default(); + let err = fm.unmarshal_msg(&buf).expect_err("absurd version count must fail to decode"); + assert!(err.to_string().contains("version count"), "unexpected error: {err}"); + } + + /// Regression test for rustfs/rustfs#2715: a corrupted per-version binary + /// length must yield a decode error instead of a giant allocation. + #[test] + fn test_unmarshal_rejects_absurd_version_header_length() { + let mut meta = Vec::new(); + rmp::encode::write_uint(&mut meta, XL_HEADER_VERSION as u64).unwrap(); + rmp::encode::write_uint(&mut meta, XL_META_VERSION as u64).unwrap(); + rmp::encode::write_sint(&mut meta, 1).unwrap(); + // One version whose header claims to be u32::MAX bytes long. + meta.push(0xc6); // bin32 + meta.extend_from_slice(&u32::MAX.to_be_bytes()); + + let buf = build_xl_buffer(&meta); + let mut fm = FileMeta::default(); + let err = fm + .unmarshal_msg(&buf) + .expect_err("absurd version header length must fail to decode"); + assert!(err.to_string().contains("version header length"), "unexpected error: {err}"); + } + #[test] fn test_new_file_meta() { let mut fm = FileMeta::new(); diff --git a/crates/filemeta/src/filemeta/codec.rs b/crates/filemeta/src/filemeta/codec.rs index c41f3ba3b..9df426a0c 100644 --- a/crates/filemeta/src/filemeta/codec.rs +++ b/crates/filemeta/src/filemeta/codec.rs @@ -179,6 +179,22 @@ impl FileMeta { self.meta_ver = meta_ver; + // `versions_len` is decoded from a potentially corrupted buffer. + // Every version contributes at least two msgpack bin headers to + // `meta`, so a count larger than the remaining bytes is corrupt; + // reject it before sizing any allocation from it (rustfs/rustfs#2715). + if versions_len > meta.len() { + error!( + "corrupt xl.meta: version count {} exceeds remaining metadata size {}", + versions_len, + meta.len() + ); + return Err(Error::other(format!( + "corrupt xl.meta: version count {versions_len} exceeds metadata size {}", + meta.len() + ))); + } + self.versions = Vec::with_capacity(versions_len); let mut cur: Cursor<&[u8]> = Cursor::new(meta); @@ -188,6 +204,11 @@ impl FileMeta { Error::other(format!("failed to read binary length for version header: {e}")) })? as usize; + let remaining = meta.len().saturating_sub(cur.position() as usize); + if bin_len > remaining { + error!("corrupt xl.meta: version header length {} exceeds remaining {} bytes", bin_len, remaining); + return Err(Error::other("corrupt xl.meta: version header length exceeds metadata size")); + } let mut header_buf = vec![0u8; bin_len]; cur.read_exact(&mut header_buf)?; @@ -203,6 +224,14 @@ impl FileMeta { Error::other(format!("failed to read binary length for version metadata: {e}")) })? as usize; + let remaining = meta.len().saturating_sub(cur.position() as usize); + if bin_len > remaining { + error!( + "corrupt xl.meta: version metadata length {} exceeds remaining {} bytes", + bin_len, remaining + ); + return Err(Error::other("corrupt xl.meta: version metadata length exceeds metadata size")); + } let mut ver_meta_buf = vec![0u8; bin_len]; cur.read_exact(&mut ver_meta_buf)?; @@ -242,14 +271,18 @@ impl FileMeta { let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; - let header_buf = &buf[start..end]; + let header_buf = buf + .get(start..end) + .ok_or_else(|| Error::other("corrupt xl.meta: version header segment out of range"))?; cur.set_position(end as u64); let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; - let ver_meta_buf = &buf[start..end]; + let ver_meta_buf = buf + .get(start..end) + .ok_or_else(|| Error::other("corrupt xl.meta: version metadata segment out of range"))?; cur.set_position(end as u64); diff --git a/crates/filemeta/src/filemeta/msgp_decode.rs b/crates/filemeta/src/filemeta/msgp_decode.rs index b7f9606b6..b99e6a7ed 100644 --- a/crates/filemeta/src/filemeta/msgp_decode.rs +++ b/crates/filemeta/src/filemeta/msgp_decode.rs @@ -16,6 +16,46 @@ use crate::{Error, Result}; use rmp::Marker; use std::io::Read; +/// Maximum accepted length for a single length-prefixed msgpack element +/// (map key, string, binary blob, ext payload, or a whole serialized +/// xl.meta record inside a metacache stream). +/// +/// Legitimate elements are far smaller: object keys are at most a few KiB +/// and even an xl.meta record fully populated up to the 10,000-version +/// object cap serializes to a few MiB (metacache streams carry xl.meta +/// without inline data), so 16 MiB leaves ample headroom. A larger value +/// means the length prefix itself is corrupt; decoding must fail with an +/// error instead of attempting a huge allocation that aborts the whole +/// process (see rustfs/rustfs#2715). +pub(crate) const MAX_MSGP_ELEMENT_SIZE: usize = 16 << 20; + +/// Reads exactly `len` bytes into a fresh buffer, treating `len` as +/// untrusted input: lengths above [`MAX_MSGP_ELEMENT_SIZE`] and allocation +/// failures surface as decode errors instead of aborting the process. +pub(crate) fn read_exact_vec(rd: &mut R, len: usize) -> Result> { + if len > MAX_MSGP_ELEMENT_SIZE { + return Err(Error::other(format!( + "corrupt msgpack element: length {len} exceeds the {MAX_MSGP_ELEMENT_SIZE} byte limit" + ))); + } + let mut buf = Vec::new(); + buf.try_reserve_exact(len) + .map_err(|e| Error::other(format!("msgpack element allocation of {len} bytes failed: {e}")))?; + buf.resize(len, 0); + rd.read_exact(&mut buf).map_err(Error::from)?; + Ok(buf) +} + +/// Bounds a decoded collection count when it is used only as a +/// pre-allocation hint. The decode loop still consumes exactly the decoded +/// number of elements (a corrupt count fails with an EOF decode error once +/// the input runs out); this merely keeps the speculative reservation from +/// aborting the process on an absurd count. +pub(crate) fn prealloc_hint(len: usize) -> usize { + const MAX_PREALLOC_ITEMS: usize = 4096; + len.min(MAX_PREALLOC_ITEMS) +} + /// Reader that prepends a single byte to the stream. Used when we've read the marker /// and need to pass it to a decoder that expects to read the marker itself. pub(crate) struct PrependByteReader<'a, R> { @@ -207,8 +247,42 @@ pub(crate) fn skip_msgp_value(rd: &mut R) -> Result<()> { Marker::Reserved => 0, }; if skip_len > 0 { - let mut buf = vec![0u8; skip_len]; - rd.read_exact(&mut buf).map_err(Error::from)?; + // Discard the payload without allocating a buffer sized by the + // untrusted length; a truncated stream surfaces as UnexpectedEof. + let copied = std::io::copy(&mut rd.by_ref().take(skip_len as u64), &mut std::io::sink()).map_err(Error::from)?; + if copied != skip_len as u64 { + return Err(Error::from(std::io::Error::new( + std::io::ErrorKind::UnexpectedEof, + "truncated msgpack value", + ))); + } } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn read_exact_vec_rejects_oversized_length() { + let mut rd: &[u8] = &[]; + let err = read_exact_vec(&mut rd, MAX_MSGP_ELEMENT_SIZE + 1).expect_err("oversized length must be rejected"); + assert!(err.to_string().contains("exceeds"), "unexpected error: {err}"); + } + + #[test] + fn read_exact_vec_reads_exact_payload() { + let mut rd: &[u8] = b"hello"; + assert_eq!(read_exact_vec(&mut rd, 5).unwrap(), b"hello"); + } + + #[test] + fn skip_msgp_value_rejects_truncated_huge_payload_without_allocating() { + // bin32 claiming u32::MAX bytes with no payload behind it. + let mut data = vec![0xc6]; + data.extend_from_slice(&u32::MAX.to_be_bytes()); + let mut rd = data.as_slice(); + assert!(skip_msgp_value(&mut rd).is_err()); + } +} diff --git a/crates/filemeta/src/filemeta/version.rs b/crates/filemeta/src/filemeta/version.rs index 44ef9e25a..208ef3875 100644 --- a/crates/filemeta/src/filemeta/version.rs +++ b/crates/filemeta/src/filemeta/version.rs @@ -22,7 +22,9 @@ //! Do NOT use `FileMetaVersion::default()` + `unmarshal_msg()` directly, as that fails on //! legacy (rmp_serde) format. `try_from` falls back to rmp_serde when hand-written decode fails. -use super::msgp_decode::{PrependByteReader, read_nil_or_array_len, read_nil_or_map_len, skip_msgp_value}; +use super::msgp_decode::{ + PrependByteReader, prealloc_hint, read_exact_vec, read_nil_or_array_len, read_nil_or_map_len, skip_msgp_value, +}; use super::*; use crate::ChecksumInfo; use rustfs_utils::HashAlgorithm; @@ -42,16 +44,13 @@ const MSGPACK_TIME_EXT_OFFICIAL: i8 = -1; fn read_msgp_string(rd: &mut R) -> Result { let len = rmp::decode::read_str_len(rd)? as usize; - let mut buf = vec![0u8; len]; - rd.read_exact(&mut buf)?; + let buf = read_exact_vec(rd, len)?; Ok(String::from_utf8(buf)?) } fn read_msgp_bin(rd: &mut R) -> Result> { let len = rmp::decode::read_bin_len(rd)? as usize; - let mut buf = vec![0u8; len]; - rd.read_exact(&mut buf)?; - Ok(buf) + read_exact_vec(rd, len) } fn deserialize_legacy_uuid_bytes<'de, D>(deserializer: D) -> std::result::Result, D::Error> @@ -183,8 +182,7 @@ fn read_msgp_time(rd: &mut R) -> Result { other => return Err(Error::other(format!("unsupported msgpack time marker: 0x{other:02x}"))), }; - let mut payload = vec![0u8; len]; - rd.read_exact(&mut payload)?; + let payload = read_exact_vec(rd, len)?; decode_msgp_time_payload(ext_type, &payload) } @@ -362,8 +360,7 @@ impl FileMetaVersion { fields -= 1; let key_len = rmp::decode::read_str_len(&mut cur)? as usize; - let mut key_buf = vec![0u8; key_len]; - cur.read_exact(&mut key_buf)?; + let key_buf = read_exact_vec(&mut cur, key_len)?; let key = String::from_utf8(key_buf)?; match key.as_str() { @@ -394,8 +391,7 @@ impl FileMetaVersion { obj_fields -= 1; let obj_key_len = rmp::decode::read_str_len(&mut prepend)? as usize; - let mut obj_key_buf = vec![0u8; obj_key_len]; - prepend.read_exact(&mut obj_key_buf)?; + let obj_key_buf = read_exact_vec(&mut prepend, obj_key_len)?; let obj_key = String::from_utf8(obj_key_buf)?; if obj_key == "DDir" { @@ -493,8 +489,7 @@ impl FileMetaVersion { fields -= 1; let key_len = rmp::decode::read_str_len(rd)?; - let mut key_buf = vec![0u8; key_len as usize]; - rd.read_exact(&mut key_buf)?; + let key_buf = read_exact_vec(rd, key_len as usize)?; let key = String::from_utf8(key_buf)?; match key.as_str() { @@ -1307,7 +1302,7 @@ impl MetaObjectV1 { "Parts" => { let len = rmp::decode::read_array_len(rd)? as usize; self.parts.clear(); - self.parts.reserve(len); + self.parts.reserve(prealloc_hint(len)); for _ in 0..len { let mut part = MetaObjectV1Part::default(); part.decode_from(rd)?; @@ -1424,7 +1419,7 @@ impl MetaObjectV1Erasure { "Distribution" => { let len = rmp::decode::read_array_len(rd)? as usize; self.distribution.clear(); - self.distribution.reserve(len); + self.distribution.reserve(prealloc_hint(len)); for _ in 0..len { self.distribution.push(rmp::decode::read_int::(rd)? as usize); } @@ -1432,7 +1427,7 @@ impl MetaObjectV1Erasure { "Checksums" => { let len = rmp::decode::read_array_len(rd)? as usize; self.checksums.clear(); - self.checksums.reserve(len); + self.checksums.reserve(prealloc_hint(len)); for _ in 0..len { let mut checksum = MetaObjectV1ChecksumInfo::default(); checksum.decode_from(rd)?; @@ -1484,7 +1479,7 @@ impl MetaObjectV1Part { "i" => self.index = Some(Bytes::from(read_msgp_bin(rd)?)), "crc" => { let len = rmp::decode::read_map_len(rd)? as usize; - let mut checksums = HashMap::with_capacity(len); + let mut checksums = HashMap::with_capacity(prealloc_hint(len)); for _ in 0..len { checksums.insert(read_msgp_string(rd)?, read_msgp_string(rd)?); } @@ -1700,9 +1695,8 @@ impl MetaObject { tracing::error!(error = %e, "decode_from: read_str_len key failed"); e })?; - let mut key_buf = vec![0u8; key_len as usize]; - rd.read_exact(&mut key_buf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact key_buf failed"); + let key_buf = read_exact_vec(rd, key_len as usize).map_err(|e| { + tracing::error!(error = %e, "decode_from: read key_buf failed"); e })?; let key = String::from_utf8(key_buf).map_err(|e| { @@ -1778,7 +1772,7 @@ impl MetaObject { e })? as usize; self.erasure_dist.clear(); - self.erasure_dist.reserve(len); + self.erasure_dist.reserve(prealloc_hint(len)); for _ in 0..len { let v: i64 = rmp::decode::read_int(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_int EcDist item failed"); @@ -1800,7 +1794,7 @@ impl MetaObject { e })? as usize; self.part_numbers.clear(); - self.part_numbers.reserve(len); + self.part_numbers.reserve(prealloc_hint(len)); for _ in 0..len { let v: i64 = rmp::decode::read_int(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_int PartNums item failed"); @@ -1821,15 +1815,14 @@ impl MetaObject { Some(n) => n, }; self.part_etags.clear(); - self.part_etags.reserve(len); + self.part_etags.reserve(prealloc_hint(len)); for _ in 0..len { let s_len = rmp::decode::read_str_len(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_str_len PartETags item failed"); e })?; - let mut sbuf = vec![0u8; s_len as usize]; - rd.read_exact(&mut sbuf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact PartETags sbuf failed"); + let sbuf = read_exact_vec(rd, s_len as usize).map_err(|e| { + tracing::error!(error = %e, "decode_from: read PartETags sbuf failed"); e })?; let s = String::from_utf8(sbuf).map_err(|e| { @@ -1845,7 +1838,7 @@ impl MetaObject { e })? as usize; self.part_sizes.clear(); - self.part_sizes.reserve(len); + self.part_sizes.reserve(prealloc_hint(len)); for _ in 0..len { let v: i64 = rmp::decode::read_int(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_int PartSizes item failed"); @@ -1866,7 +1859,7 @@ impl MetaObject { Some(n) => n, }; self.part_actual_sizes.clear(); - self.part_actual_sizes.reserve(len); + self.part_actual_sizes.reserve(prealloc_hint(len)); for _ in 0..len { let v: i64 = rmp::decode::read_int(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_int PartASizes item failed"); @@ -1881,15 +1874,14 @@ impl MetaObject { e })? as usize; self.part_indices.clear(); - self.part_indices.reserve(len); + self.part_indices.reserve(prealloc_hint(len)); for _ in 0..len { let blen = rmp::decode::read_bin_len(rd).map_err(|e| { tracing::error!(error = %e, "decode_from: read_bin_len PartIdx item failed"); e })? as usize; - let mut buf = vec![0u8; blen]; - rd.read_exact(&mut buf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact PartIdx buf failed"); + let buf = read_exact_vec(rd, blen).map_err(|e| { + tracing::error!(error = %e, "decode_from: read PartIdx buf failed"); e })?; self.part_indices.push(Bytes::from(buf)); @@ -1933,9 +1925,8 @@ impl MetaObject { tracing::error!(error = %e, "decode_from: read_str_len MetaSys key failed"); e })?; - let mut kbuf = vec![0u8; k_len as usize]; - rd.read_exact(&mut kbuf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact MetaSys kbuf failed"); + let kbuf = read_exact_vec(rd, k_len as usize).map_err(|e| { + tracing::error!(error = %e, "decode_from: read MetaSys kbuf failed"); e })?; let k = String::from_utf8(kbuf).map_err(|e| { @@ -1947,9 +1938,8 @@ impl MetaObject { tracing::error!(error = %e, "decode_from: read_bin_len MetaSys value failed"); e })? as usize; - let mut v = vec![0u8; blen]; - rd.read_exact(&mut v).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact MetaSys value failed"); + let v = read_exact_vec(rd, blen).map_err(|e| { + tracing::error!(error = %e, "decode_from: read MetaSys value failed"); e })?; @@ -1973,9 +1963,8 @@ impl MetaObject { tracing::error!(error = %e, "decode_from: read_str_len MetaUsr key failed"); e })?; - let mut kbuf = vec![0u8; k_len as usize]; - rd.read_exact(&mut kbuf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact MetaUsr kbuf failed"); + let kbuf = read_exact_vec(rd, k_len as usize).map_err(|e| { + tracing::error!(error = %e, "decode_from: read MetaUsr kbuf failed"); e })?; let k = String::from_utf8(kbuf).map_err(|e| { @@ -1987,9 +1976,8 @@ impl MetaObject { tracing::error!(error = %e, "decode_from: read_str_len MetaUsr value failed"); e })?; - let mut vbuf = vec![0u8; v_len as usize]; - rd.read_exact(&mut vbuf).map_err(|e| { - tracing::error!(error = %e, "decode_from: read_exact MetaUsr vbuf failed"); + let vbuf = read_exact_vec(rd, v_len as usize).map_err(|e| { + tracing::error!(error = %e, "decode_from: read MetaUsr vbuf failed"); e })?; let v = String::from_utf8(vbuf).map_err(|e| { @@ -2446,8 +2434,7 @@ impl MetaDeleteMarker { fields -= 1; let key_len = rmp::decode::read_str_len(rd)?; - let mut key_buf = vec![0u8; key_len as usize]; - rd.read_exact(&mut key_buf)?; + let key_buf = read_exact_vec(rd, key_len as usize)?; let key = String::from_utf8(key_buf)?; match key.as_str() { @@ -2472,13 +2459,11 @@ impl MetaDeleteMarker { self.meta_sys.clear(); for _ in 0..len { let k_len = rmp::decode::read_str_len(rd)?; - let mut kbuf = vec![0u8; k_len as usize]; - rd.read_exact(&mut kbuf)?; + let kbuf = read_exact_vec(rd, k_len as usize)?; let k = String::from_utf8(kbuf)?; let blen = rmp::decode::read_bin_len(rd)? as usize; - let mut v = vec![0u8; blen]; - rd.read_exact(&mut v)?; + let v = read_exact_vec(rd, blen)?; self.meta_sys.insert(k, v); } diff --git a/crates/filemeta/src/filemeta_inline.rs b/crates/filemeta/src/filemeta_inline.rs index d66910cb0..658433e1e 100644 --- a/crates/filemeta/src/filemeta_inline.rs +++ b/crates/filemeta/src/filemeta_inline.rs @@ -13,8 +13,9 @@ // limitations under the License. use crate::error::{Error, Result}; +use crate::filemeta::msgp_decode::{prealloc_hint, read_exact_vec}; use serde::{Deserialize, Serialize}; -use std::io::{Cursor, Read}; +use std::io::Cursor; use uuid::Uuid; #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] @@ -41,14 +42,20 @@ impl InlineData { let str_len = rmp::decode::read_str_len(&mut scan_cur)? as usize; let key_start = scan_cur.position() as usize; let key_end = key_start + str_len; + let key = buf + .get(key_start..key_end) + .ok_or_else(|| Error::other("InlineData key out of range"))?; scan_cur.set_position(key_end as u64); let bin_len = rmp::decode::read_bin_len(&mut scan_cur)? as usize; let value_start = scan_cur.position() as usize; let value_end = value_start + bin_len; + if value_end > buf.len() { + return Err(Error::other("InlineData value out of range")); + } scan_cur.set_position(value_end as u64); - if should_remove(&buf[key_start..key_end]) { + if should_remove(key) { return Ok(true); } } @@ -67,20 +74,22 @@ impl InlineData { let mut cur = Cursor::new(buf); let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize; - let mut keys = Vec::with_capacity(fields_len); - let mut values = Vec::with_capacity(fields_len); + let mut keys = Vec::with_capacity(prealloc_hint(fields_len)); + let mut values = Vec::with_capacity(prealloc_hint(fields_len)); let mut found = false; while fields_len > 0 { fields_len -= 1; let str_len = rmp::decode::read_str_len(&mut cur)? as usize; - let mut field_buf = vec![0u8; str_len]; - cur.read_exact(&mut field_buf)?; + let field_buf = read_exact_vec(&mut cur, str_len)?; let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; + let value = buf + .get(start..end) + .ok_or_else(|| Error::other("InlineData value out of range"))?; cur.set_position(end as u64); if should_remove(field_buf.as_slice()) { @@ -89,7 +98,7 @@ impl InlineData { } keys.push(String::from_utf8(field_buf)?); - values.push(buf[start..end].to_vec()); + values.push(value.to_vec()); } if !found { @@ -114,20 +123,22 @@ impl InlineData { let same = first_key == second_key; let mut cur = Cursor::new(buf); let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize; - let mut keys = Vec::with_capacity(fields_len + 1); - let mut values = Vec::with_capacity(fields_len + 1); + let mut keys = Vec::with_capacity(prealloc_hint(fields_len) + 1); + let mut values = Vec::with_capacity(prealloc_hint(fields_len) + 1); let mut found = false; while fields_len > 0 { fields_len -= 1; let str_len = rmp::decode::read_str_len(&mut cur)? as usize; - let mut field_buf = vec![0u8; str_len]; - cur.read_exact(&mut field_buf)?; + let field_buf = read_exact_vec(&mut cur, str_len)?; let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; + let value = buf + .get(start..end) + .ok_or_else(|| Error::other("InlineData value out of range"))?; cur.set_position(end as u64); let should_remove = if same { @@ -142,7 +153,7 @@ impl InlineData { } keys.push(String::from_utf8(field_buf)?); - values.push(buf[start..end].to_vec()); + values.push(value.to_vec()); } if !found { @@ -209,20 +220,20 @@ impl InlineData { let str_len = rmp::decode::read_str_len(&mut cur)?; - let mut field_buff = vec![0u8; str_len as usize]; - - cur.read_exact(&mut field_buff)?; + let field_buff = read_exact_vec(&mut cur, str_len as usize)?; let field = String::from_utf8(field_buff)?; let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; + let value = buf + .get(start..end) + .ok_or_else(|| Error::other("InlineData value out of range"))?; cur.set_position(end as u64); if field.as_str() == key { - let buf = &buf[start..end]; - return Ok(Some(buf.to_vec())); + return Ok(Some(value.to_vec())); } } @@ -234,7 +245,8 @@ impl InlineData { return Ok(()); } - let mut cur = Cursor::new(self.after_version()); + let buf = self.after_version(); + let mut cur = Cursor::new(buf); let mut fields_len = rmp::decode::read_map_len(&mut cur)?; @@ -243,9 +255,7 @@ impl InlineData { let str_len = rmp::decode::read_str_len(&mut cur)?; - let mut field_buff = vec![0u8; str_len as usize]; - - cur.read_exact(&mut field_buff)?; + let field_buff = read_exact_vec(&mut cur, str_len as usize)?; let field = String::from_utf8(field_buff)?; if field.is_empty() { @@ -255,6 +265,9 @@ impl InlineData { let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; + if end > buf.len() { + return Err(Error::other("InlineData value out of range")); + } cur.set_position(end as u64); } @@ -276,8 +289,8 @@ impl InlineData { let mut cur = Cursor::new(buf); let mut fields_len = rmp::decode::read_map_len(&mut cur)? as usize; - let mut keys = Vec::with_capacity(fields_len + 1); - let mut values = Vec::with_capacity(fields_len + 1); + let mut keys = Vec::with_capacity(prealloc_hint(fields_len) + 1); + let mut values = Vec::with_capacity(prealloc_hint(fields_len) + 1); let mut replaced = false; @@ -286,19 +299,18 @@ impl InlineData { let str_len = rmp::decode::read_str_len(&mut cur)?; - let mut field_buff = vec![0u8; str_len as usize]; - - cur.read_exact(&mut field_buff)?; + let field_buff = read_exact_vec(&mut cur, str_len as usize)?; let find_key = String::from_utf8(field_buff)?; let bin_len = rmp::decode::read_bin_len(&mut cur)? as usize; let start = cur.position() as usize; let end = start + bin_len; + let find_value = buf + .get(start..end) + .ok_or_else(|| Error::other("InlineData value out of range"))?; cur.set_position(end as u64); - let find_value = &buf[start..end]; - if find_key.as_str() == key { values.push(value.clone()); replaced = true diff --git a/crates/filemeta/src/metacache.rs b/crates/filemeta/src/metacache.rs index b44a14254..e311a6bd6 100644 --- a/crates/filemeta/src/metacache.rs +++ b/crates/filemeta/src/metacache.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::filemeta::msgp_decode::MAX_MSGP_ELEMENT_SIZE; use crate::{ Error, FileInfo, FileInfoOpts, FileInfoVersions, FileMeta, FileMetaShallowVersion, Result, VersionType, get_file_info, merge_file_meta_versions, merge_file_meta_versions_with_write_quorum, @@ -638,25 +639,36 @@ impl MetacacheReader { } pub async fn read_more(&mut self, read_size: usize) -> Result<&[u8]> { - let ext_size = read_size + self.offset; - - let extra = ext_size - self.offset; - if self.buf.capacity() >= ext_size { - // Extend the buffer if we have enough space. - self.buf.resize(ext_size, 0); - } else { - self.buf.extend(vec![0u8; extra]); + // `read_size` is usually a length decoded from the stream itself, so + // it is untrusted: a corrupted length prefix must yield a decode + // error instead of a huge allocation that aborts the process + // (see rustfs/rustfs#2715). + if read_size > MAX_MSGP_ELEMENT_SIZE { + let err = Error::other(format!( + "metacache stream corrupt: element length {read_size} exceeds the {MAX_MSGP_ELEMENT_SIZE} byte limit" + )); + self.err = Some(err.clone()); + return Err(err); } let pref = self.offset; + let ext_size = pref + read_size; + + if self.buf.len() < ext_size { + let extra = ext_size - self.buf.len(); + if let Err(e) = self.buf.try_reserve(extra) { + let err = Error::other(format!("metacache stream: buffer allocation of {extra} bytes failed: {e}")); + self.err = Some(err.clone()); + return Err(err); + } + self.buf.resize(ext_size, 0); + } self.rd.read_exact(&mut self.buf[pref..ext_size]).await?; self.offset += read_size; - let data = &self.buf[pref..ext_size]; - - Ok(data) + Ok(&self.buf[pref..ext_size]) } fn reset(&mut self) { @@ -990,6 +1002,74 @@ mod tests { assert_eq!(objs, nobjs); } + fn corrupt_stream_with_metadata_len(len_marker: u8, len: u32) -> Vec { + let mut data = Vec::new(); + rmp::encode::write_u8(&mut data, METACACHE_STREAM_VERSION).unwrap(); + rmp::encode::write_bool(&mut data, true).unwrap(); + rmp::encode::write_str(&mut data, "object").unwrap(); + // Hand-written bin/str length prefix claiming an absurd payload size. + data.push(len_marker); + data.extend_from_slice(&len.to_be_bytes()); + data + } + + /// Regression test for rustfs/rustfs#2715: a corrupted metadata length + /// prefix must surface as a decode error instead of attempting a huge + /// allocation that aborts the process. + #[tokio::test] + async fn test_reader_rejects_corrupt_bin_length_prefix() { + // 0xc6 = bin32 marker. + let data = corrupt_stream_with_metadata_len(0xc6, u32::MAX); + let mut r = MetacacheReader::new(Cursor::new(data)); + let err = r.peek().await.expect_err("corrupt bin length prefix must fail to decode"); + assert!(err.to_string().contains("exceeds"), "error should mention the exceeded limit, got: {err}"); + + // The reader must stay in the error state instead of retrying. + assert!(r.peek().await.is_err()); + } + + #[tokio::test] + async fn test_reader_rejects_corrupt_str_length_prefix() { + let mut data = Vec::new(); + rmp::encode::write_u8(&mut data, METACACHE_STREAM_VERSION).unwrap(); + rmp::encode::write_bool(&mut data, true).unwrap(); + // 0xdb = str32 marker with an absurd object-name length. + data.push(0xdb); + data.extend_from_slice(&u32::MAX.to_be_bytes()); + + let mut r = MetacacheReader::new(Cursor::new(data)); + let err = r.peek().await.expect_err("corrupt str length prefix must fail to decode"); + assert!(err.to_string().contains("exceeds"), "error should mention the exceeded limit, got: {err}"); + } + + #[tokio::test] + async fn test_reader_skip_rejects_corrupt_length_prefix() { + let data = corrupt_stream_with_metadata_len(0xc6, u32::MAX); + let mut r = MetacacheReader::new(Cursor::new(data)); + assert!(r.skip(1).await.is_err(), "skip over a corrupt length prefix must fail"); + } + + /// Large-but-legitimate records (well above typical sizes, below the + /// corruption guard) must still round-trip. + #[tokio::test] + async fn test_reader_accepts_large_legitimate_metadata() { + let entry = MetaCacheEntry { + name: "big-object".to_string(), + metadata: vec![0xab; 4 << 20], + cached: None, + reusable: false, + }; + + let mut f = Cursor::new(Vec::new()); + let mut w = MetacacheWriter::new(&mut f); + w.write(std::slice::from_ref(&entry)).await.unwrap(); + w.close().await.unwrap(); + + let mut r = MetacacheReader::new(Cursor::new(f.into_inner())); + let decoded = r.read_all().await.unwrap(); + assert_eq!(decoded, vec![entry]); + } + #[test] fn file_info_versions_with_free_versions_includes_persisted_tier_cleanup_records() { let version_id = Uuid::new_v4();