Fix SszBlock bug relating to excess ssz

If you gave SszBlock too many bytes it would hash them too.
This commit is contained in:
Paul Hauner 2018-09-20 13:51:58 +10:00
parent 7020a08b7d
commit 55ce4998d8
No known key found for this signature in database
GPG Key ID: 303E4494BB28068C
2 changed files with 37 additions and 20 deletions

View File

@ -1,15 +1,12 @@
use super::utils::types::Hash256; use super::utils::types::Hash256;
use super::attestation_record::{ use super::attestation_record::AttestationRecord;
AttestationRecord,
MIN_SSZ_ATTESTION_RECORD_LENGTH,
};
use super::ssz::{ Encodable, SszStream }; use super::ssz::{ Encodable, SszStream };
pub const MIN_SSZ_BLOCK_LENGTH: usize = { pub const MIN_SSZ_BLOCK_LENGTH: usize = {
4 + 32 + // parent_hash 4 + 32 + // parent_hash
8 + // slot_number 8 + // slot_number
4 + 32 + // randao_reveal 4 + 32 + // randao_reveal
4 + MIN_SSZ_ATTESTION_RECORD_LENGTH + // attestations (minimum one) 4 + // attestations (assuming zero)
4 + 32 + // pow_chain_ref 4 + 32 + // pow_chain_ref
4 + 32 + // active_state_root 4 + 32 + // active_state_root
4 + 32 // crystallized_state_root 4 + 32 // crystallized_state_root
@ -71,8 +68,7 @@ mod tests {
#[test] #[test]
pub fn test_block_min_ssz_length() { pub fn test_block_min_ssz_length() {
let mut b = Block::zero(); let b = Block::zero();
b.attestations = vec![AttestationRecord::zero()];
let mut ssz_stream = SszStream::new(); let mut ssz_stream = SszStream::new();
ssz_stream.append(&b); ssz_stream.append(&b);

View File

@ -7,6 +7,7 @@ use super::block::{
MIN_SSZ_BLOCK_LENGTH, MIN_SSZ_BLOCK_LENGTH,
MAX_SSZ_BLOCK_LENGTH, MAX_SSZ_BLOCK_LENGTH,
}; };
use super::attestation_record::MIN_SSZ_ATTESTION_RECORD_LENGTH;
#[derive(Debug, PartialEq)] #[derive(Debug, PartialEq)]
pub enum BlockValidatorError { pub enum BlockValidatorError {
@ -32,34 +33,41 @@ impl<'a> SszBlock<'a> {
pub fn from_slice(vec: &'a [u8]) pub fn from_slice(vec: &'a [u8])
-> Result<Self, BlockValidatorError> -> Result<Self, BlockValidatorError>
{ {
let ssz = &vec[..]; let untrimmed_ssz = &vec[..];
let len = vec.len();
/* /*
* Ensure the SSZ is long enough to be a block. * Ensure the SSZ is long enough to be a block with
* one attestation record (not necessarily a valid
* attestation record).
*/ */
if len < MIN_SSZ_BLOCK_LENGTH { if vec.len() < MIN_SSZ_BLOCK_LENGTH + MIN_SSZ_ATTESTION_RECORD_LENGTH {
return Err(BlockValidatorError::TooShort); return Err(BlockValidatorError::TooShort);
} }
/* /*
* Ensure the SSZ slice isn't longer than is possible for a block. * Ensure the SSZ slice isn't longer than is possible for a block.
*/ */
if len > MAX_SSZ_BLOCK_LENGTH { if vec.len() > MAX_SSZ_BLOCK_LENGTH {
return Err(BlockValidatorError::TooLong); return Err(BlockValidatorError::TooLong);
} }
/* /*
* Determine how many bytes are used to store attestation records * Determine how many bytes are used to store attestation records.
* and ensure that length is enough to store at least one attestation
* record.
*/ */
let attestation_len = decode_length(ssz, 80, LENGTH_BYTES) let attestation_len = decode_length(untrimmed_ssz, 80, LENGTH_BYTES)
.map_err(|_| BlockValidatorError::TooShort)?; .map_err(|_| BlockValidatorError::TooShort)?;
if len < (76 + attestation_len + 96) { /*
* The block only has one variable field, `attestations`, therefore
* the size of the block must be the minimum size, plus the length
* of the attestations.
*/
let block_ssz_len = {
MIN_SSZ_BLOCK_LENGTH + attestation_len
};
if vec.len() < block_ssz_len {
return Err(BlockValidatorError::TooShort); return Err(BlockValidatorError::TooShort);
} }
Ok(Self{ Ok(Self{
ssz, ssz: &untrimmed_ssz[0..block_ssz_len],
attestation_len, attestation_len,
len, len: block_ssz_len,
}) })
} }
@ -156,6 +164,19 @@ mod tests {
); );
} }
#[test]
fn test_ssz_block_single_attestation_record_one_byte_long() {
let mut b = Block::zero();
b.attestations = vec![AttestationRecord::zero()];
let mut ssz = get_block_ssz(&b);
let original_len = ssz.len();
ssz.push(42);
let ssz_block = SszBlock::from_slice(&ssz[..]).unwrap();
assert_eq!(ssz_block.len, original_len);
}
#[test] #[test]
fn test_ssz_block_single_attestation_record() { fn test_ssz_block_single_attestation_record() {
let mut b = Block::zero(); let mut b = Block::zero();