diff --git a/CHANGELOG.md b/CHANGELOG.md index a93d220f..499d93bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Restricted RLP decoding to match the RLP spec and disallow leading zeros ([#335]) + +[#335]: https://github.com/recmo/uint/pulls/335 + ## [1.11.0] - 2023-10-27 ### Added diff --git a/src/support/alloy_rlp.rs b/src/support/alloy_rlp.rs index 27b10476..50ebb976 100644 --- a/src/support/alloy_rlp.rs +++ b/src/support/alloy_rlp.rs @@ -13,7 +13,7 @@ const MAX_BITS: usize = 55 * 8; /// Allows a [`Uint`] to be serialized as RLP. /// -/// See +/// See impl Encodable for Uint { #[inline] fn length(&self) -> usize { @@ -72,11 +72,24 @@ impl Encodable for Uint { /// Allows a [`Uint`] to be deserialized from RLP. /// -/// See +/// See impl Decodable for Uint { #[inline] fn decode(buf: &mut &[u8]) -> Result { let bytes = Header::decode_bytes(buf, false)?; + + // The RLP spec states that deserialized positive integers with leading zeroes + // get treated as invalid. + // + // See: + // https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp/ + // + // To check this, we only need to check if the first byte is zero to make sure + // there are no leading zeros + if !bytes.is_empty() && bytes[0] == 0 { + return Err(Error::LeadingZero); + } + Self::try_from_be_slice(bytes).ok_or(Error::Overflow) } } @@ -148,4 +161,30 @@ mod test { }); }); } + + #[test] + fn test_invalid_uints() { + // these are non-canonical because they have leading zeros + assert_eq!( + U256::decode(&mut &hex!("820000")[..]), + Err(Error::LeadingZero) + ); + // 00 is not a valid uint + // See https://github.com/ethereum/go-ethereum/blob/cd2953567268777507b1ec29269315324fb5aa9c/rlp/decode_test.go#L118 + assert_eq!(U256::decode(&mut &hex!("00")[..]), Err(Error::LeadingZero)); + // these are non-canonical because they can fit in a single byte, i.e. + // 0x7f, 0x33 + assert_eq!( + U256::decode(&mut &hex!("8100")[..]), + Err(Error::NonCanonicalSingleByte) + ); + assert_eq!( + U256::decode(&mut &hex!("817f")[..]), + Err(Error::NonCanonicalSingleByte) + ); + assert_eq!( + U256::decode(&mut &hex!("8133")[..]), + Err(Error::NonCanonicalSingleByte) + ); + } } diff --git a/src/support/fastrlp.rs b/src/support/fastrlp.rs index c2ab8a3e..c8551833 100644 --- a/src/support/fastrlp.rs +++ b/src/support/fastrlp.rs @@ -13,7 +13,7 @@ const MAX_BITS: usize = 55 * 8; /// Allows a [`Uint`] to be serialized as RLP. /// -/// See +/// See impl Encodable for Uint { #[inline] fn length(&self) -> usize { @@ -72,16 +72,31 @@ impl Encodable for Uint { /// Allows a [`Uint`] to be deserialized from RLP. /// -/// See +/// See impl Decodable for Uint { #[inline] fn decode(buf: &mut &[u8]) -> Result { + // let bytes = Header::decode_bytes(buf, false)?; let header = Header::decode(buf)?; if header.list { return Err(DecodeError::UnexpectedList); } + let bytes = &buf[..header.payload_length]; *buf = &buf[header.payload_length..]; + + // The RLP spec states that deserialized positive integers with leading zeroes + // get treated as invalid. + // + // See: + // https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp/ + // + // To check this, we only need to check if the first byte is zero to make sure + // there are no leading zeros + if !bytes.is_empty() && bytes[0] == 0 { + return Err(DecodeError::LeadingZero); + } + Self::try_from_be_slice(bytes).ok_or(DecodeError::Overflow) } }