From 68cfc6b300340a5030ce436e8fb059eda90b4b7e Mon Sep 17 00:00:00 2001 From: Joel DSouza Date: Fri, 10 Jul 2026 16:22:22 +0200 Subject: [PATCH] fix(serder)!: guard version-string fixed widths and validate at every deserialize entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects on the serder edge: - VersionString::to_str() rendered major/minor/size with unguarded {:x}/ {:06x}: any value beyond its fixed hex width (major/minor > 0xF, size > 0xFFFFFF) silently widened the documented 17-byte version string and corrupted the frame. to_str() now returns Result and rejects overflow with the new SerderError::VersionStringOverflow variant. - All five event serializers mapped the measured-length u32 conversion failure into SerderError::DigestError (wrong failure domain) and never checked the 6-hex-digit size capacity; a >16 MiB event serialized to a corrupt frame. They now reject it as VersionStringOverflow. - Only deserialize_event validated the version string; the per-ilk public entry points (deserialize_inception/rotation/interaction/ delegated_inception/delegated_rotation) skipped it, so raw whose length contradicts its version-string size — e.g. whitespace-padded JSON with an intact SAID — was accepted. Every public deserializer now calls validate_version_string first. BREAKING CHANGE: VersionString::to_str() returns Result; SerderError gains the VersionStringOverflow variant. Bug probes (red before fix): to_str_rejects_size_beyond_fixed_width, to_str_rejects_{major,minor}_beyond_one_hex_digit, serialize_ixn_rejects_event_beyond_version_size_capacity, all_public_deserializers_reject_length_mismatched_raw. Co-Authored-By: Claude Fable 5 --- cesr/src/serder/deserialize.rs | 168 +++++++++++++++++++++++++++++-- cesr/src/serder/error.rs | 11 ++ cesr/src/serder/serialize/dip.rs | 15 ++- cesr/src/serder/serialize/drt.rs | 15 ++- cesr/src/serder/serialize/icp.rs | 15 ++- cesr/src/serder/serialize/ixn.rs | 40 +++++++- cesr/src/serder/serialize/rot.rs | 15 ++- cesr/src/serder/version.rs | 98 ++++++++++++++++-- 8 files changed, 335 insertions(+), 42 deletions(-) diff --git a/cesr/src/serder/deserialize.rs b/cesr/src/serder/deserialize.rs index 7030538..38ef8ae 100644 --- a/cesr/src/serder/deserialize.rs +++ b/cesr/src/serder/deserialize.rs @@ -65,9 +65,11 @@ pub fn deserialize_event(raw: &[u8]) -> Result { /// /// # Errors /// -/// Returns [`SerderError`] if JSON parsing fails, required fields are missing -/// or invalid, or the SAID does not verify. +/// Returns [`SerderError`] if JSON parsing fails, the version string is +/// malformed or inconsistent with the input length, required fields are +/// missing or invalid, or the SAID does not verify. pub fn deserialize_inception(raw: &[u8]) -> Result { + validate_version_string(raw)?; let val: Value = serde_json::from_slice(raw)?; let digest_code = infer_digest_code(get_str(&val, "d")?)?; let d_str = get_str(&val, "d")?; @@ -110,9 +112,11 @@ pub fn deserialize_inception(raw: &[u8]) -> Result /// /// # Errors /// -/// Returns [`SerderError`] if JSON parsing fails, required fields are missing -/// or invalid, or the SAID does not verify. +/// Returns [`SerderError`] if JSON parsing fails, the version string is +/// malformed or inconsistent with the input length, required fields are +/// missing or invalid, or the SAID does not verify. pub fn deserialize_rotation(raw: &[u8]) -> Result { + validate_version_string(raw)?; let val: Value = serde_json::from_slice(raw)?; let digest_code = infer_digest_code(get_str(&val, "d")?)?; @@ -156,9 +160,11 @@ pub fn deserialize_rotation(raw: &[u8]) -> Result { /// /// # Errors /// -/// Returns [`SerderError`] if JSON parsing fails, required fields are missing -/// or invalid, or the SAID does not verify. +/// Returns [`SerderError`] if JSON parsing fails, the version string is +/// malformed or inconsistent with the input length, required fields are +/// missing or invalid, or the SAID does not verify. pub fn deserialize_interaction(raw: &[u8]) -> Result { + validate_version_string(raw)?; let val: Value = serde_json::from_slice(raw)?; let digest_code = infer_digest_code(get_str(&val, "d")?)?; @@ -186,9 +192,11 @@ pub fn deserialize_interaction(raw: &[u8]) -> Result Result { + validate_version_string(raw)?; let val: Value = serde_json::from_slice(raw)?; let digest_code = infer_digest_code(get_str(&val, "d")?)?; let d_str = get_str(&val, "d")?; @@ -235,8 +243,9 @@ pub fn deserialize_delegated_inception(raw: &[u8]) -> Result Result { let rotation = deserialize_rotation(raw)?; Ok(DelegatedRotationEvent::new(rotation)) @@ -1298,6 +1307,145 @@ mod tests { assert_eq!((n, d), (1, 1)); } + // ----------------------------------------------------------------------- + // Version-string validation at every public entry point + // ----------------------------------------------------------------------- + + /// Insert one space after the first comma: the parsed JSON (and therefore + /// the SAID computed over its compact re-serialization) is unchanged, but + /// the raw length no longer matches the version-string size field. + fn whitespace_padded(raw: &[u8]) -> Vec { + let idx = raw + .iter() + .position(|b| *b == b',') + .expect("event JSON has a comma"); + let mut padded = Vec::with_capacity(raw.len() + 1); + padded.extend_from_slice(&raw[..=idx]); + padded.push(b' '); + padded.extend_from_slice(&raw[idx + 1..]); + padded + } + + fn probe_icp() -> InceptionEvent { + InceptionEvent::new( + make_prefixer().into(), + Seqner::new(0), + make_saider(), + vec![make_verfer()], + Tholder::Simple(1), + vec![make_diger()], + Tholder::Simple(1), + vec![], + 0, + vec![], + vec![], + ) + } + + fn probe_rot() -> RotationEvent { + RotationEvent::new( + make_prefixer().into(), + Seqner::new(1), + make_saider(), + make_saider(), + vec![make_verfer()], + Tholder::Simple(1), + vec![make_diger()], + Tholder::Simple(1), + vec![], + vec![], + 0, + vec![], + vec![], + ) + } + + #[test] + fn deserialize_inception_rejects_length_mismatched_raw() { + let raw = serialize_inception(&probe_icp()).unwrap(); + let padded = whitespace_padded(raw.as_bytes()); + // Precondition making this a real probe: the padded bytes are still + // valid JSON with an intact SAID — only the length lies. + assert!(serde_json::from_slice::(&padded).is_ok()); + assert!( + matches!( + deserialize_inception(&padded), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_inception must reject raw whose length contradicts its version string" + ); + } + + #[test] + fn deserialize_event_rejects_length_mismatched_raw() { + let raw = serialize_inception(&probe_icp()).unwrap(); + let padded = whitespace_padded(raw.as_bytes()); + assert!( + matches!( + deserialize_event(&padded), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_event must keep rejecting length-mismatched raw" + ); + } + + #[test] + fn deserialize_rotation_rejects_length_mismatched_raw() { + let raw = serialize_rotation(&probe_rot()).unwrap(); + assert!( + matches!( + deserialize_rotation(&whitespace_padded(raw.as_bytes())), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_rotation must reject length-mismatched raw" + ); + } + + #[test] + fn deserialize_interaction_rejects_length_mismatched_raw() { + let event = InteractionEvent::new( + make_prefixer().into(), + Seqner::new(1), + make_saider(), + make_saider(), + vec![], + ); + let raw = serialize_interaction(&event).unwrap(); + assert!( + matches!( + deserialize_interaction(&whitespace_padded(raw.as_bytes())), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_interaction must reject length-mismatched raw" + ); + } + + #[test] + fn deserialize_delegated_inception_rejects_length_mismatched_raw() { + let event = DelegatedInceptionEvent::new(probe_icp(), make_prefixer().into()); + let raw = serialize_delegated_inception(&event).unwrap(); + assert!( + matches!( + deserialize_delegated_inception(&whitespace_padded(raw.as_bytes())), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_delegated_inception must reject length-mismatched raw" + ); + } + + #[test] + fn deserialize_delegated_rotation_rejects_length_mismatched_raw() { + let event = DelegatedRotationEvent::new(probe_rot()); + let raw = serialize_delegated_rotation(&event).unwrap(); + assert!( + matches!( + deserialize_delegated_rotation(&whitespace_padded(raw.as_bytes())), + Err(SerderError::InvalidVersionString(_)) + ), + "deserialize_delegated_rotation must reject length-mismatched raw" + ); + } + // ----------------------------------------------------------------------- // Parse-failure routing: a malformed qb64 code is a parsing-domain error // ----------------------------------------------------------------------- diff --git a/cesr/src/serder/error.rs b/cesr/src/serder/error.rs index e63b4a1..aea868c 100644 --- a/cesr/src/serder/error.rs +++ b/cesr/src/serder/error.rs @@ -56,6 +56,17 @@ pub enum SerderError { source: ParsingError, }, + /// A version-string field's value does not fit its fixed-width hex + /// encoding — rendering it anyway would widen the string and corrupt the + /// 17-byte frame. + #[error("version string field '{field}' exceeds its fixed-width capacity of {max}")] + VersionStringOverflow { + /// The version-string field that does not fit. + field: &'static str, + /// The largest value the field's fixed width can encode. + max: u32, + }, + /// Digest computation failed. #[error("digest error: {0}")] DigestError(String), diff --git a/cesr/src/serder/serialize/dip.rs b/cesr/src/serder/serialize/dip.rs index a6259d4..01d8305 100644 --- a/cesr/src/serder/serialize/dip.rs +++ b/cesr/src/serder/serialize/dip.rs @@ -14,7 +14,7 @@ use super::{SerializedEvent, matters_to_json_array, seal_to_json, tholder_to_jso use crate::serder::error::SerderError; use crate::serder::primitives::{identifier_to_qb64_string, sn_to_hex, to_qb64_string}; use crate::serder::said::{compute_digest, said_placeholder}; -use crate::serder::version::VersionString; +use crate::serder::version::{VERSION_SIZE_MAX, VersionString}; /// Serialize a [`DelegatedInceptionEvent`] to canonical JSON with a computed SAID. /// @@ -71,14 +71,19 @@ pub fn serialize_delegated_inception( delegator: &delegator_qb64, }; - let phase1_vs = VersionString::keri_json_v1().to_str(); + let phase1_vs = VersionString::keri_json_v1().to_str()?; let phase1_json = build_dip_json(&phase1_vs, &placeholder, &fields)?; - let measured_len = - u32::try_from(phase1_json.len()).map_err(|e| SerderError::DigestError(e.to_string()))?; + let measured_len = u32::try_from(phase1_json.len()) + .ok() + .filter(|len| *len <= VERSION_SIZE_MAX) + .ok_or(SerderError::VersionStringOverflow { + field: "size", + max: VERSION_SIZE_MAX, + })?; let vs_with_size = VersionString::keri_json_v1() .with_size(measured_len) - .to_str(); + .to_str()?; let phase2_json = build_dip_json(&vs_with_size, &placeholder, &fields)?; let said = compute_digest(phase2_json.as_bytes(), digest_code)?; diff --git a/cesr/src/serder/serialize/drt.rs b/cesr/src/serder/serialize/drt.rs index 6adbf83..0dc76ea 100644 --- a/cesr/src/serder/serialize/drt.rs +++ b/cesr/src/serder/serialize/drt.rs @@ -14,7 +14,7 @@ use super::{SerializedEvent, matters_to_json_array, seal_to_json, tholder_to_jso use crate::serder::error::SerderError; use crate::serder::primitives::{identifier_to_qb64_string, sn_to_hex, to_qb64_string}; use crate::serder::said::{compute_digest, said_placeholder}; -use crate::serder::version::VersionString; +use crate::serder::version::{VERSION_SIZE_MAX, VersionString}; /// Serialize a [`DelegatedRotationEvent`] to canonical JSON with a computed SAID. /// @@ -67,14 +67,19 @@ pub fn serialize_delegated_rotation( anchors: &anchors_value, }; - let phase1_vs = VersionString::keri_json_v1().to_str(); + let phase1_vs = VersionString::keri_json_v1().to_str()?; let phase1_json = build_drt_json(&phase1_vs, &placeholder, &fields)?; - let measured_len = - u32::try_from(phase1_json.len()).map_err(|e| SerderError::DigestError(e.to_string()))?; + let measured_len = u32::try_from(phase1_json.len()) + .ok() + .filter(|len| *len <= VERSION_SIZE_MAX) + .ok_or(SerderError::VersionStringOverflow { + field: "size", + max: VERSION_SIZE_MAX, + })?; let vs_with_size = VersionString::keri_json_v1() .with_size(measured_len) - .to_str(); + .to_str()?; let phase2_json = build_drt_json(&vs_with_size, &placeholder, &fields)?; let said = compute_digest(phase2_json.as_bytes(), digest_code)?; diff --git a/cesr/src/serder/serialize/icp.rs b/cesr/src/serder/serialize/icp.rs index 5bd12f3..b095593 100644 --- a/cesr/src/serder/serialize/icp.rs +++ b/cesr/src/serder/serialize/icp.rs @@ -14,7 +14,7 @@ use super::{SerializedEvent, matters_to_json_array, seal_to_json, tholder_to_jso use crate::serder::error::SerderError; use crate::serder::primitives::{sn_to_hex, to_qb64_string}; use crate::serder::said::{compute_digest, said_placeholder}; -use crate::serder::version::VersionString; +use crate::serder::version::{VERSION_SIZE_MAX, VersionString}; /// Serialize an [`InceptionEvent`] to canonical JSON with a computed SAID. /// @@ -64,15 +64,20 @@ pub fn serialize_inception(event: &InceptionEvent) -> Result Result = (0..340_000) + .map(|_| Seal::Digest { d: make_saider() }) + .collect(); + let event = InteractionEvent::new( + make_prefixer().into(), + Seqner::new(1), + make_saider(), + make_saider(), + anchors, + ); + let result = serialize_interaction(&event); + assert!(matches!( + result, + Err(SerderError::VersionStringOverflow { + field: "size", + max: VERSION_SIZE_MAX, + }) + )); + } + #[test] fn serialize_ixn_version_string_size_matches() { let event = make_event(); diff --git a/cesr/src/serder/serialize/rot.rs b/cesr/src/serder/serialize/rot.rs index fe9da7f..9cc1c80 100644 --- a/cesr/src/serder/serialize/rot.rs +++ b/cesr/src/serder/serialize/rot.rs @@ -14,7 +14,7 @@ use super::{SerializedEvent, matters_to_json_array, seal_to_json, tholder_to_jso use crate::serder::error::SerderError; use crate::serder::primitives::{identifier_to_qb64_string, sn_to_hex, to_qb64_string}; use crate::serder::said::{compute_digest, said_placeholder}; -use crate::serder::version::VersionString; +use crate::serder::version::{VERSION_SIZE_MAX, VersionString}; /// Serialize a [`RotationEvent`] to canonical JSON with a computed SAID. /// @@ -62,14 +62,19 @@ pub fn serialize_rotation(event: &RotationEvent) -> Result String { - format!( + /// + /// # Errors + /// + /// Returns [`SerderError::VersionStringOverflow`] if `major`, `minor`, or + /// `size` does not fit its fixed-width hex field — rendering anyway would + /// silently widen the string and break the 17-byte frame every parser + /// depends on. + pub fn to_str(&self) -> Result { + if self.major > VERSION_DIGIT_MAX { + return Err(SerderError::VersionStringOverflow { + field: "major", + max: u32::from(VERSION_DIGIT_MAX), + }); + } + if self.minor > VERSION_DIGIT_MAX { + return Err(SerderError::VersionStringOverflow { + field: "minor", + max: u32::from(VERSION_DIGIT_MAX), + }); + } + if self.size > VERSION_SIZE_MAX { + return Err(SerderError::VersionStringOverflow { + field: "size", + max: VERSION_SIZE_MAX, + }); + } + Ok(format!( "{}{:x}{:x}{}{:06x}_", self.proto.as_str(), self.major, self.minor, self.kind.as_str(), self.size, - ) + )) } /// Parse a version string from the first 17 bytes of `input`. @@ -248,13 +278,67 @@ mod tests { #[test] fn to_str_zero_size() { let vs = VersionString::keri_json_v1(); - assert_eq!(vs.to_str(), "KERI10JSON000000_"); + assert_eq!(vs.to_str().unwrap(), "KERI10JSON000000_"); } #[test] fn to_str_nonzero_size() { let vs = VersionString::keri_json_v1().with_size(0x25d); - assert_eq!(vs.to_str(), "KERI10JSON00025d_"); + assert_eq!(vs.to_str().unwrap(), "KERI10JSON00025d_"); + } + + #[test] + fn to_str_renders_max_size_at_fixed_width() { + let vs = VersionString::keri_json_v1().with_size(VERSION_SIZE_MAX); + let rendered = vs.to_str().unwrap(); + assert_eq!(rendered, "KERI10JSONffffff_"); + assert_eq!(rendered.len(), VERSION_STRING_LEN); + } + + #[test] + fn to_str_rejects_size_beyond_fixed_width() { + // Bug probe: {:06x} silently widened to 7 hex digits for sizes above + // VERSION_SIZE_MAX, corrupting the 17-byte frame instead of erroring. + let vs = VersionString::keri_json_v1().with_size(VERSION_SIZE_MAX + 1); + assert!(matches!( + vs.to_str().unwrap_err(), + SerderError::VersionStringOverflow { + field: "size", + max: VERSION_SIZE_MAX, + } + )); + } + + #[test] + fn to_str_renders_max_versions_at_fixed_width() { + let vs = VersionString::new(Protocol::Keri, 0xF, 0xF, SerKind::Json, 0); + let rendered = vs.to_str().unwrap(); + assert_eq!(rendered, "KERIffJSON000000_"); + assert_eq!(rendered.len(), VERSION_STRING_LEN); + } + + #[test] + fn to_str_rejects_major_beyond_one_hex_digit() { + let vs = VersionString::new(Protocol::Keri, 0x10, 0, SerKind::Json, 0); + assert!(matches!( + vs.to_str().unwrap_err(), + SerderError::VersionStringOverflow { field: "major", .. } + )); + } + + #[test] + fn to_str_rejects_minor_beyond_one_hex_digit() { + let vs = VersionString::new(Protocol::Keri, 0, 0x10, SerKind::Json, 0); + assert!(matches!( + vs.to_str().unwrap_err(), + SerderError::VersionStringOverflow { field: "minor", .. } + )); + } + + #[test] + fn size_capacity_matches_size_field_width() { + let width = u32::try_from(SIZE_LEN).unwrap(); + assert_eq!(VERSION_SIZE_MAX, (1_u32 << (4 * width)) - 1); } #[test] @@ -270,7 +354,7 @@ mod tests { #[test] fn parse_roundtrip() { let original = VersionString::new(Protocol::Acdc, 2, 5, SerKind::Cbor, 0x001a_2b3c); - let rendered = original.to_str(); + let rendered = original.to_str().unwrap(); let parsed = VersionString::parse(&rendered).unwrap(); assert_eq!(original, parsed); }