From fd2f9d0d15b3f0ab8b2f31551679822ed5a372d5 Mon Sep 17 00:00:00 2001 From: Kirk Baird Date: Tue, 26 Mar 2019 16:45:25 +1100 Subject: [PATCH] Remove ssz encoding of length from; Signature, AggregateSiganture, PublicKey, SecretKey --- eth2/utils/bls/src/aggregate_signature.rs | 26 ++++++++++------------- eth2/utils/bls/src/lib.rs | 2 ++ eth2/utils/bls/src/public_key.rs | 24 ++++++++++----------- eth2/utils/bls/src/secret_key.rs | 16 ++++++++------ eth2/utils/bls/src/signature.rs | 17 +++++++-------- eth2/utils/boolean-bitfield/src/lib.rs | 3 --- eth2/utils/ssz/src/decode.rs | 1 + 7 files changed, 43 insertions(+), 46 deletions(-) diff --git a/eth2/utils/bls/src/aggregate_signature.rs b/eth2/utils/bls/src/aggregate_signature.rs index 783a86ebb7..585584545f 100644 --- a/eth2/utils/bls/src/aggregate_signature.rs +++ b/eth2/utils/bls/src/aggregate_signature.rs @@ -4,8 +4,8 @@ use bls_aggregates::{ }; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; -use serde_hex::{encode as hex_encode, PrefixedHexVisitor}; -use ssz::{decode_ssz_list, hash, Decodable, DecodeError, Encodable, SszStream, TreeHash}; +use serde_hex::{encode as hex_encode, HexVisitor}; +use ssz::{decode, hash, Decodable, DecodeError, Encodable, SszStream, TreeHash}; /// A BLS aggregate signature. /// @@ -121,22 +121,18 @@ impl AggregateSignature { impl Encodable for AggregateSignature { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.as_bytes()); + s.append_encoded_raw(&self.as_bytes()); } } impl Decodable for AggregateSignature { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { - let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let raw_sig = - RawAggregateSignature::from_bytes(&sig_bytes).map_err(|_| DecodeError::Invalid)?; - Ok(( - Self { - aggregate_signature: raw_sig, - is_empty: false, - }, - i, - )) + if bytes.len() - i < BLS_AGG_SIG_BYTE_SIZE { + return Err(DecodeError::TooShort); + } + let agg_sig = AggregateSignature::from_bytes(&bytes[i..(i + BLS_AGG_SIG_BYTE_SIZE)]) + .map_err(|_| DecodeError::Invalid)?; + Ok((agg_sig, i + BLS_AGG_SIG_BYTE_SIZE)) } } @@ -156,8 +152,8 @@ impl<'de> Deserialize<'de> for AggregateSignature { where D: Deserializer<'de>, { - let bytes = deserializer.deserialize_str(PrefixedHexVisitor)?; - let agg_sig = AggregateSignature::from_bytes(&bytes[..]) + let bytes = deserializer.deserialize_str(HexVisitor)?; + let agg_sig = decode(&bytes[..]) .map_err(|e| serde::de::Error::custom(format!("invalid ssz ({:?})", e)))?; Ok(agg_sig) } diff --git a/eth2/utils/bls/src/lib.rs b/eth2/utils/bls/src/lib.rs index b8fb2157f2..57c463ace3 100644 --- a/eth2/utils/bls/src/lib.rs +++ b/eth2/utils/bls/src/lib.rs @@ -17,6 +17,8 @@ pub use crate::signature::Signature; pub const BLS_AGG_SIG_BYTE_SIZE: usize = 96; pub const BLS_SIG_BYTE_SIZE: usize = 96; +pub const BLS_SECRET_KEY_BYTE_SIZE: usize = 48; +pub const BLS_PUBLIC_KEY_BYTE_SIZE: usize = 48; use hashing::hash; use ssz::ssz_encode; diff --git a/eth2/utils/bls/src/public_key.rs b/eth2/utils/bls/src/public_key.rs index 346bb7de38..98ff40d71b 100644 --- a/eth2/utils/bls/src/public_key.rs +++ b/eth2/utils/bls/src/public_key.rs @@ -1,12 +1,9 @@ -use super::SecretKey; +use super::{SecretKey, BLS_PUBLIC_KEY_BYTE_SIZE}; use bls_aggregates::PublicKey as RawPublicKey; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; -use serde_hex::{encode as hex_encode, PrefixedHexVisitor}; -use ssz::{ - decode, decode_ssz_list, hash, ssz_encode, Decodable, DecodeError, Encodable, SszStream, - TreeHash, -}; +use serde_hex::{encode as hex_encode, HexVisitor}; +use ssz::{decode, hash, ssz_encode, Decodable, DecodeError, Encodable, SszStream, TreeHash}; use std::default; use std::hash::{Hash, Hasher}; @@ -64,15 +61,18 @@ impl default::Default for PublicKey { impl Encodable for PublicKey { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.0.as_bytes()); + s.append_encoded_raw(&self.0.as_bytes()); } } impl Decodable for PublicKey { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { - let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let raw_sig = RawPublicKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; - Ok((PublicKey(raw_sig), i)) + if bytes.len() - i < BLS_PUBLIC_KEY_BYTE_SIZE { + return Err(DecodeError::TooShort); + } + let raw_sig = RawPublicKey::from_bytes(&bytes[i..(i + BLS_PUBLIC_KEY_BYTE_SIZE)]) + .map_err(|_| DecodeError::TooShort)?; + Ok((PublicKey(raw_sig), i + BLS_PUBLIC_KEY_BYTE_SIZE)) } } @@ -90,8 +90,8 @@ impl<'de> Deserialize<'de> for PublicKey { where D: Deserializer<'de>, { - let bytes = deserializer.deserialize_str(PrefixedHexVisitor)?; - let pubkey = PublicKey::from_bytes(&bytes[..]) + let bytes = deserializer.deserialize_str(HexVisitor)?; + let pubkey = decode(&bytes[..]) .map_err(|e| serde::de::Error::custom(format!("invalid pubkey ({:?})", e)))?; Ok(pubkey) } diff --git a/eth2/utils/bls/src/secret_key.rs b/eth2/utils/bls/src/secret_key.rs index 6e57f8e1e9..40c4695131 100644 --- a/eth2/utils/bls/src/secret_key.rs +++ b/eth2/utils/bls/src/secret_key.rs @@ -1,11 +1,10 @@ +use super::BLS_SECRET_KEY_BYTE_SIZE; use bls_aggregates::{DecodeError as BlsDecodeError, SecretKey as RawSecretKey}; use hex::encode as hex_encode; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use serde_hex::HexVisitor; -use ssz::{ - decode, decode_ssz_list, ssz_encode, Decodable, DecodeError, Encodable, SszStream, TreeHash, -}; +use ssz::{decode, ssz_encode, Decodable, DecodeError, Encodable, SszStream, TreeHash}; /// A single BLS signature. /// @@ -34,15 +33,18 @@ impl SecretKey { impl Encodable for SecretKey { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.0.as_bytes()); + s.append_encoded_raw(&self.0.as_bytes()); } } impl Decodable for SecretKey { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { - let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let raw_sig = RawSecretKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?; - Ok((SecretKey(raw_sig), i)) + if bytes.len() - i < BLS_SECRET_KEY_BYTE_SIZE { + return Err(DecodeError::TooShort); + } + let raw_sig = RawSecretKey::from_bytes(&bytes[i..(i + BLS_SECRET_KEY_BYTE_SIZE)]) + .map_err(|_| DecodeError::TooShort)?; + Ok((SecretKey(raw_sig), i + BLS_SECRET_KEY_BYTE_SIZE)) } } diff --git a/eth2/utils/bls/src/signature.rs b/eth2/utils/bls/src/signature.rs index e1a146981d..d19af545f0 100644 --- a/eth2/utils/bls/src/signature.rs +++ b/eth2/utils/bls/src/signature.rs @@ -4,10 +4,7 @@ use hex::encode as hex_encode; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use serde_hex::HexVisitor; -use ssz::{ - decode, decode_ssz_list, hash, ssz_encode, Decodable, DecodeError, Encodable, SszStream, - TreeHash, -}; +use ssz::{decode, hash, ssz_encode, Decodable, DecodeError, Encodable, SszStream, TreeHash}; /// A single BLS signature. /// @@ -103,15 +100,17 @@ impl Signature { impl Encodable for Signature { fn ssz_append(&self, s: &mut SszStream) { - s.append_vec(&self.as_bytes()); + s.append_encoded_raw(&self.as_bytes()); } } impl Decodable for Signature { fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> { - let (sig_bytes, i) = decode_ssz_list(bytes, i)?; - let signature = Signature::from_bytes(&sig_bytes)?; - Ok((signature, i)) + if bytes.len() - i < BLS_SIG_BYTE_SIZE { + return Err(DecodeError::TooShort); + } + let signature = Signature::from_bytes(&bytes[i..(i + BLS_SIG_BYTE_SIZE)])?; + Ok((signature, i + BLS_SIG_BYTE_SIZE)) } } @@ -138,7 +137,7 @@ impl<'de> Deserialize<'de> for Signature { D: Deserializer<'de>, { let bytes = deserializer.deserialize_str(HexVisitor)?; - let signature = Signature::from_bytes(&bytes[..]) + let signature = decode(&bytes[..]) .map_err(|e| serde::de::Error::custom(format!("invalid ssz ({:?})", e)))?; Ok(signature) } diff --git a/eth2/utils/boolean-bitfield/src/lib.rs b/eth2/utils/boolean-bitfield/src/lib.rs index 78e5020738..ac637b3aa4 100644 --- a/eth2/utils/boolean-bitfield/src/lib.rs +++ b/eth2/utils/boolean-bitfield/src/lib.rs @@ -145,10 +145,7 @@ impl std::ops::BitAnd for BooleanBitfield { } impl Encodable for BooleanBitfield { -<<<<<<< HEAD -======= // ssz_append encodes Self according to the `ssz` spec. ->>>>>>> v0.5.0-state-transition-tests fn ssz_append(&self, s: &mut ssz::SszStream) { s.append_vec(&self.to_bytes()) } diff --git a/eth2/utils/ssz/src/decode.rs b/eth2/utils/ssz/src/decode.rs index efc8c38db8..7ed6fe4916 100644 --- a/eth2/utils/ssz/src/decode.rs +++ b/eth2/utils/ssz/src/decode.rs @@ -15,6 +15,7 @@ pub trait Decodable: Sized { /// /// The single ssz encoded value/container/list will be decoded as the given type, /// by recursively calling `ssz_decode`. +/// Check on totality for underflowing the length of bytes and overflow checks done per container pub fn decode(ssz_bytes: &[u8]) -> Result<(T), DecodeError> where T: Decodable,