diff --git a/crates/didbot-data/src/cid.rs b/crates/didbot-data/src/cid.rs index dd794f69..91cd317e 100644 --- a/crates/didbot-data/src/cid.rs +++ b/crates/didbot-data/src/cid.rs @@ -321,6 +321,14 @@ fn write_varint(out: &mut Vec, mut value: u64) { /// /// Nine continuation bytes is already more than a `u64` holds, so the length /// cap is what stops a hostile string from being read forever. +/// +/// The encoding must also be minimal, which multiformats requires of an +/// unsigned varint and which is not a tidiness here. A [`Cid`] keeps the bytes +/// it was read from and prints from them, so a version written `81 00` rather +/// than `01` is a second byte sequence, a second string and a second `Cid` +/// that compares unequal — for one block, with one digest. The rule is that +/// the last group of a multi-byte varint has to carry something, so a +/// terminating zero group after a continuation byte is refused. fn read_varint(input: &mut &[u8]) -> Result { let mut value: u64 = 0; for index in 0..10 { @@ -332,6 +340,9 @@ fn read_varint(input: &mut &[u8]) -> Result { .filter(|shifted| shifted >> (index * 7) == payload) .ok_or(CidError::BadVarint)?; if byte & 0x80 == 0 { + if index > 0 && payload == 0 { + return Err(CidError::BadVarint); + } return Ok(value); } } @@ -420,6 +431,55 @@ mod tests { } } + /// A varint written wider than it needs gives one block two names. + /// + /// Asserted as the round trip rather than as an error kind: a `Cid` keeps + /// the bytes it read and prints from them, so anything `from_bytes` + /// accepts must serialise back to what it was handed. The named error + /// comes after, so a regression fails on the property and not on the + /// spelling of the refusal. + #[test] + fn a_padded_varint_would_be_a_second_name_for_one_block() { + let canonical = Cid::of_dag_cbor(&[0xa0]); + // Version 1 written `81 00`; codec, multihash and digest unchanged. + let mut padded = vec![0x81, 0x00]; + padded.extend_from_slice(&canonical.as_bytes()[1..]); + if let Ok(read) = Cid::from_bytes(&padded) { + assert_eq!( + read.to_string(), + canonical.to_string(), + "{read} is a second name for the block {canonical} names" + ); + } + assert_eq!(Cid::from_bytes(&padded), Err(CidError::BadVarint)); + + // The same padding in the codec, which a reader checks by value and + // is least likely to check the width of. + let mut padded_codec = vec![0x01, 0xf1, 0x00]; + padded_codec.extend_from_slice(&canonical.as_bytes()[2..]); + assert_eq!(Cid::from_bytes(&padded_codec), Err(CidError::BadVarint)); + + // The canonical spelling still reads, so the rule refuses a form + // rather than the field. + assert_eq!(Cid::from_bytes(canonical.as_bytes()), Ok(canonical)); + } + + /// The same refusal reaches a link inside a block, which is where a + /// padded CID would actually arrive: a peer's record or tree node. + #[test] + fn a_link_carrying_a_padded_cid_is_not_a_link() { + let canonical = Cid::of_dag_cbor(&[0xa0]); + let mut padded = vec![0x81, 0x00]; + padded.extend_from_slice(&canonical.as_bytes()[1..]); + // Tag 42 over a byte string of the identity prefix and those bytes. + let mut block = vec![0xd8, 0x2a, 0x58, (padded.len() + 1) as u8, 0x00]; + block.extend_from_slice(&padded); + assert!( + crate::dag_cbor::decode(&block).is_err(), + "a block linking through a padded cid decoded" + ); + } + #[test] fn a_varint_with_no_end_is_refused_rather_than_read_forever() { let mut input: &[u8] = &[0x80; 12];