diff --git a/packages/rs-dpp/src/asset_lock/reduced_asset_lock_value/mod.rs b/packages/rs-dpp/src/asset_lock/reduced_asset_lock_value/mod.rs index ac4c17608f1..2983bd18973 100644 --- a/packages/rs-dpp/src/asset_lock/reduced_asset_lock_value/mod.rs +++ b/packages/rs-dpp/src/asset_lock/reduced_asset_lock_value/mod.rs @@ -23,7 +23,12 @@ pub use v0::{AssetLockValueGettersV0, AssetLockValueSettersV0}; serde::Serialize, serde::Deserialize, )] -#[platform_serialize(unversioned)] +// Stored asset-lock values are decoded from GroveDB proof elements on the +// client before the quorum signature is checked, so the byte budget must be +// enforced by the decoder itself. A valid value is well under 1 KiB (P2PKH +// script, at most `max_asset_lock_usage_attempts` 32-byte tags); the limit +// leaves room for Core's 10,000-byte script ceiling. +#[platform_serialize(limit = 15000, unversioned)] #[serde(tag = "$formatVersion")] pub enum AssetLockValue { #[serde(rename = "0")] @@ -225,3 +230,63 @@ mod json_convertible_tests { assert_eq!(original, recovered); } } + +#[cfg(test)] +mod deserialize_limit_tests { + use super::*; + use crate::serialization::{PlatformDeserializable, PlatformSerializable}; + + /// Bincode-encode the V0 shape by hand so the `tx_out_script` length prefix + /// can claim more bytes than exist in the payload. + fn payload_with_script_length(fake_len: u64) -> Vec { + let config = bincode::config::standard() + .with_big_endian() + .with_no_limit(); + let mut buf = Vec::new(); + // enum discriminant: V0 + buf.extend_from_slice(&bincode::encode_to_vec(0u32, config).unwrap()); + // initial_credit_value + buf.extend_from_slice(&bincode::encode_to_vec(1_000u64, config).unwrap()); + // tx_out_script length prefix, with no bytes following it + buf.extend_from_slice(&bincode::encode_to_vec(fake_len, config).unwrap()); + buf + } + + /// A proof element is untrusted input: a length prefix must be rejected + /// against the byte budget before it sizes an allocation. Without the + /// limit this was `vec.resize(8_000_000_000, 0)` and an abort. + #[test] + fn rejects_script_length_prefix_beyond_budget_without_allocating() { + let payload = payload_with_script_length(8_000_000_000); + let err = AssetLockValue::deserialize_from_bytes(&payload) + .expect_err("oversized length prefix must be rejected"); + assert!( + matches!(err, ProtocolError::MaxEncodedBytesReachedError { .. }), + "unexpected error: {err}" + ); + } + + /// The largest value the server can legitimately store must stay inside + /// the budget on both the encode and decode side, or the node could fail + /// to persist it. + #[test] + fn largest_valid_value_round_trips_under_limit() { + let platform_version = PlatformVersion::latest(); + let max_tags = platform_version + .drive_abci + .validation_and_processing + .state_transitions + .max_asset_lock_usage_attempts as usize; + let original = AssetLockValue::new( + u64::MAX, + vec![0xffu8; 10_000], + u64::MAX, + vec![Bytes32::new([0xff; 32]); max_tags], + platform_version, + ) + .expect("value"); + let bytes = original.serialize_to_bytes().expect("serialize"); + let recovered = AssetLockValue::deserialize_from_bytes(&bytes).expect("deserialize"); + assert_eq!(original, recovered); + } +} diff --git a/packages/rs-dpp/src/data_contract/document_type/property/mod.rs b/packages/rs-dpp/src/data_contract/document_type/property/mod.rs index 8ab8617eef1..3a48552bf84 100644 --- a/packages/rs-dpp/src/data_contract/document_type/property/mod.rs +++ b/packages/rs-dpp/src/data_contract/document_type/property/mod.rs @@ -645,24 +645,41 @@ impl DocumentPropertyType { } } + /// Reads exactly `len` bytes. The length comes from the (possibly + /// untrusted) serialized document itself, so it must never size an + /// allocation: the buffer grows only as bytes actually arrive, and a + /// prefix that claims more than the document holds is rejected once the + /// input runs out. + fn read_exact_bounded( + buf: &mut BufReader<&[u8]>, + len: usize, + what: &str, + ) -> Result, DataContractError> { + let mut value = Vec::new(); + buf.by_ref() + .take(len as u64) + .read_to_end(&mut value) + .map_err(|_| { + DataContractError::CorruptedSerialization(format!( + "error reading {what} of length {len} from serialized document" + )) + })?; + if value.len() != len { + return Err(DataContractError::CorruptedSerialization(format!( + "{what} declares {len} bytes but only {} remain in the serialized document", + value.len() + ))); + } + Ok(value) + } + fn read_varint_value(buf: &mut BufReader<&[u8]>) -> Result, DataContractError> { let bytes: usize = buf.read_varint().map_err(|_| { DataContractError::CorruptedSerialization( "error reading varint length from serialized document".to_string(), ) })?; - if bytes == 0 { - Ok(vec![]) - } else { - let mut value: Vec = vec![0u8; bytes]; - buf.read_exact(&mut value).map_err(|_| { - DataContractError::CorruptedSerialization(format!( - "error reading varint of length {} from serialized document", - bytes - )) - })?; - Ok(value) - } + Self::read_exact_bounded(buf, bytes, "varint value") } /// Reads an optional value from the buffer @@ -794,13 +811,18 @@ impl DocumentPropertyType { (Some(min), Some(max)) if min == max => { // if min == max, then we don't need a varint for the length let len = min as usize; - let mut bytes = vec![0; len]; - buf.read_exact(&mut bytes).map_err(|_| { - DataContractError::DecodingContractError(DecodingError::new(format!( - "expected to read {} bytes (min size for byte array)", - len - ))) - })?; + // Schema-bounded (u16), so never an allocation hazard; routed + // through the bounded reader for uniformity while keeping the + // error variant this arm has always produced. + let bytes = Self::read_exact_bounded(buf, len, "fixed-size byte array") + .map_err(|_| { + DataContractError::DecodingContractError(DecodingError::new( + format!( + "expected to read {} bytes (min size for byte array)", + len + ), + )) + })?; // To save space we use predefined types for most popular blob sizes // so we don't need to store the size of the blob match bytes.len() { @@ -834,12 +856,7 @@ impl DocumentPropertyType { "error reading varint of object length".to_string(), ) })?; - let mut object_bytes = vec![0u8; object_byte_len]; - buf.read_exact(&mut object_bytes).map_err(|_| { - DataContractError::CorruptedSerialization( - "error reading object bytes".to_string(), - ) - })?; + let object_bytes = Self::read_exact_bounded(buf, object_byte_len, "object")?; // Wrap the bytes in a BufReader let mut object_buf_reader = BufReader::new(&object_bytes[..]); let mut finished_buffer = false; @@ -4239,6 +4256,71 @@ mod tests { // read_optionally_from() tests // ----------------------------------------------------------------------- + /// A serialized document is untrusted input: a length prefix must never + /// size an allocation before it has been checked against the bytes that + /// are actually present. Before this check a two-byte string field + /// declaring a multi-gigabyte length aborted the process on allocation. + #[test] + fn test_read_optionally_from_rejects_length_prefix_longer_than_input() { + use std::io::BufReader; + let prop = DocumentPropertyType::String(StringPropertySizes { + min_length: None, + max_length: None, + }); + // varint 2^62 followed by two bytes of payload + let mut data = vec![0xffu8, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x3f]; + data.extend_from_slice(b"ab"); + let mut reader = BufReader::new(data.as_slice()); + let err = prop + .read_optionally_from(&mut reader, true) + .expect_err("oversized length prefix must be rejected, not allocated"); + assert!( + err.to_string() + .contains("remain in the serialized document"), + "unexpected error: {err}" + ); + + let object = DocumentPropertyType::Object(IndexMap::new()); + let mut reader = BufReader::new(data.as_slice()); + let err = object + .read_optionally_from(&mut reader, true) + .expect_err("oversized object length must be rejected, not allocated"); + assert!( + err.to_string() + .contains("remain in the serialized document"), + "unexpected error: {err}" + ); + } + + /// The guard must not reject a prefix that exactly consumes the rest of + /// the document: that is the normal shape of a document's last field. + #[test] + fn test_read_optionally_from_accepts_length_prefix_equal_to_remaining_input() { + use std::io::BufReader; + let prop = DocumentPropertyType::String(StringPropertySizes { + min_length: None, + max_length: None, + }); + let mut data = vec![2u8]; + data.extend_from_slice(b"ab"); + let mut reader = BufReader::new(data.as_slice()); + let (value, finished) = prop + .read_optionally_from(&mut reader, true) + .expect("exact-length prefix must decode"); + assert_eq!(value, Some(Value::Text("ab".to_string()))); + assert!(!finished); + + // One byte short of the declared length is still a rejection. + let mut reader = BufReader::new(&data[..2]); + let err = prop + .read_optionally_from(&mut reader, true) + .expect_err("short input must be rejected"); + assert!( + err.to_string().contains("only 1 remain"), + "unexpected error: {err}" + ); + } + #[test] fn test_read_optionally_from_optional_marker_none() { use std::io::BufReader; diff --git a/packages/rs-dpp/src/group/group_action/mod.rs b/packages/rs-dpp/src/group/group_action/mod.rs index 2cc962059d5..d449f10cf32 100644 --- a/packages/rs-dpp/src/group/group_action/mod.rs +++ b/packages/rs-dpp/src/group/group_action/mod.rs @@ -27,7 +27,13 @@ use serde::{Deserialize, Serialize}; serde(tag = "$formatVersion") )] #[cfg_attr(feature = "value-conversion", derive(ValueConvertible))] -#[platform_serialize(unversioned)] //versioned directly, no need to use platform_version +// Stored group actions are decoded from GroveDB proof elements on the client +// before the quorum signature is checked, so the byte budget must be enforced +// by the decoder itself. Every payload (notes, config change, pricing +// schedule) is copied out of the state transition that proposed the action, +// and `StateTransition` is capped at the same 100,000 bytes, so no valid +// stored action can exceed this. +#[platform_serialize(limit = 100000, unversioned)] //versioned directly, no need to use platform_version pub enum GroupAction { #[cfg_attr(feature = "serde-conversion", serde(rename = "0"))] V0(GroupActionV0), @@ -69,3 +75,38 @@ impl GroupActionAccessors for GroupAction { // TODO(unification pass 2): add round-trip tests for GroupAction once we have an // explicit fixture (GroupActionV0 has no Default — its `event: GroupActionEvent` // field is itself a versioned enum without Default). + +#[cfg(test)] +mod deserialize_limit_tests { + use super::*; + use crate::serialization::PlatformDeserializable; + + /// A proof element is untrusted input: a note length prefix must be + /// rejected against the byte budget before it sizes an allocation. + #[test] + fn rejects_note_length_prefix_beyond_budget_without_allocating() { + let config = bincode::config::standard() + .with_big_endian() + .with_no_limit(); + let mut buf = Vec::new(); + // GroupAction::V0, then GroupActionV0 { contract_id, proposer_id, position, event } + buf.extend_from_slice(&bincode::encode_to_vec(0u32, config).unwrap()); + buf.extend_from_slice(&[0u8; 32]); + buf.extend_from_slice(&[0u8; 32]); + buf.extend_from_slice(&bincode::encode_to_vec(0u16, config).unwrap()); + // GroupActionEvent::TokenEvent(TokenEvent::Freeze(id, Some(note))) + buf.extend_from_slice(&bincode::encode_to_vec(0u32, config).unwrap()); + buf.extend_from_slice(&bincode::encode_to_vec(2u32, config).unwrap()); + buf.extend_from_slice(&[0u8; 32]); + buf.push(1); + // note length prefix claiming 8 GB, with no bytes following it + buf.extend_from_slice(&bincode::encode_to_vec(8_000_000_000u64, config).unwrap()); + + let err = GroupAction::deserialize_from_bytes(&buf) + .expect_err("oversized length prefix must be rejected"); + assert!( + matches!(err, ProtocolError::MaxEncodedBytesReachedError { .. }), + "unexpected error: {err}" + ); + } +}