From b8a8879c78d992ea553d95673dacac8bdbb71474 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Wed, 2 Sep 2026 23:48:57 -0400 Subject: [PATCH] fix(didbot-data)!: refuse a simple value below 32 in the one-byte form `f8 14` decoded as `false` and re-encoded as `f4`, so a peer could hand this server two spellings of one block and get two CIDs for it. Major type 7's one-byte form carries the extended simple range only; floats are now routed by argument width instead of by value, which also stops a simple value in that range being reported as a float. Co-Authored-By: Claude Opus 5 (1M context) --- crates/didbot-data/src/dag_cbor/read.rs | 96 +++++++++++++++++++++---- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/crates/didbot-data/src/dag_cbor/read.rs b/crates/didbot-data/src/dag_cbor/read.rs index 68df4b2c..a0462d78 100644 --- a/crates/didbot-data/src/dag_cbor/read.rs +++ b/crates/didbot-data/src/dag_cbor/read.rs @@ -19,6 +19,9 @@ //! * an argument written wider than it needs — `0x18 0x05` for 5; //! * an indefinite length, on a string, a list or a map; //! * a float, a half float included, and the `undefined` simple value; +//! * a simple value below 32 written in the one-byte form — `f8 14` is a +//! second spelling of `f4`, and a decoder that read it would re-encode the +//! block to a different CID; //! * a tag other than 42, and a tag 42 over anything but an //! identity-prefixed CID; //! * a map key that is not a text string, and a map whose keys are not in @@ -149,6 +152,12 @@ struct Header { major: u8, /// The argument, whatever width it was written in. argument: u64, + /// How many bytes the argument was written in, 0 for the inline form. + /// + /// Kept because major type 7 reads it as a *kind* rather than a width: + /// two, four and eight bytes are a half, single and double float, and one + /// byte is a simple value in the extended range. + width: usize, } impl<'a> Reader<'a> { @@ -188,10 +197,25 @@ impl<'a> Reader<'a> { // A float is a major type 7 argument and is caught below, where the // simple values are read: the widths overlap, and 25, 26 and 27 mean // "half, single, double" there rather than "a wider integer". - if major != MAJOR_SIMPLE && !shortest(argument, width) { + // + // Major type 7's one-byte form is not a width either, and it has its + // own shortest rule: it carries a simple value in the extended range, + // 32 to 255, and a value below 32 written in it is a second spelling + // of a byte that already exists. `f8 14` and `f4` would both be + // `false`, and only one of them is the block that value hashes as. + let non_canonical = if major == MAJOR_SIMPLE { + width == 1 && argument < 32 + } else { + !shortest(argument, width) + }; + if non_canonical { return Err(CborError::NonCanonicalArgument { argument, width }); } - Ok(Header { major, argument }) + Ok(Header { + major, + argument, + width, + }) } /// Reads one value at `depth`. @@ -214,7 +238,10 @@ impl<'a> Reader<'a> { MAJOR_LIST => self.list(header.argument, depth), MAJOR_MAP => self.map(header.argument, depth), MAJOR_TAG => self.link(header.argument, depth), - // 7 by elimination: three bits hold nothing else. + // 7 by elimination: three bits hold nothing else. The wider + // forms are the floats; the inline and one-byte forms are simple + // values. + _ if header.width >= 2 => Err(CborError::Float), _ => simple(header.argument), } } @@ -312,20 +339,16 @@ fn integer(argument: u64) -> Result { /// The three simple values the profile has, and the refusal of everything else. /// -/// The floats live here too: in major type 7 the arguments 25, 26 and 27 are -/// a half, single and double float rather than a wider integer, and their -/// payload has already been read as the argument by the time this is reached. +/// Only the inline and one-byte forms reach here, so the argument is a simple +/// value and never a float's payload; the floats are refused by width in +/// [`Reader::value`]. 23 is `undefined`, which the data model does not have, +/// and 32 upwards is the unassigned extended range. fn simple(argument: u64) -> Result { match argument { 20 => Ok(Value::Bool(false)), 21 => Ok(Value::Bool(true)), 22 => Ok(Value::Null), - // 23 is `undefined`, which the data model does not have; 25 to 27 are - // the floats it also does not have. The rest is unassigned. - other => match u8::try_from(other) { - Ok(value) if value < 24 => Err(CborError::Simple(value)), - _ => Err(CborError::Float), - }, + other => Err(CborError::Simple(u8::try_from(other).unwrap_or(u8::MAX))), } } @@ -405,6 +428,55 @@ mod tests { ); } + /// `f8` carries a simple value in the extended range, 32 to 255. A value + /// below 32 written in it is a second spelling of a one-byte form, and + /// accepting it would mean two blocks — two CIDs — for one value. + /// + /// This is the sharpest form of the round-trip rule in this file, so it + /// is asserted as the round trip: whatever `decode` accepts must + /// re-encode to the bytes it came from. + #[test] + fn a_simple_value_below_the_extended_range_is_not_written_in_two_bytes() { + for value in 0..32u8 { + let block = [0xf8, value]; + let Ok(decoded) = decode(&block) else { + continue; + }; + assert_eq!( + encode(&decoded), + block, + "f8 {value:02x} decoded to {decoded:?}, which re-encodes to other bytes" + ); + } + // The three that used to come back as values, named so the failure + // says which spelling was accepted rather than only that one was. + assert_eq!( + decode(&[0xf8, 20]), + Err(CborError::NonCanonicalArgument { + argument: 20, + width: 1 + }) + ); + assert_eq!( + decode(&[0xf8, 21]), + Err(CborError::NonCanonicalArgument { + argument: 21, + width: 1 + }) + ); + assert_eq!( + decode(&[0xf8, 22]), + Err(CborError::NonCanonicalArgument { + argument: 22, + width: 1 + }) + ); + // 32 upwards is the range the form is for; it is unassigned rather + // than non-canonical, and refused for being a simple value the data + // model has no room for. + assert_eq!(decode(&[0xf8, 32]), Err(CborError::Simple(32))); + } + /// Floats decode to nothing, at every width. #[test] fn floats_are_refused_at_every_width() { -- 2.51.2