From d8394bded7e98d5c7aff15a311e45ab165b6e90a Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 12 Feb 2019 15:16:23 +0100 Subject: [PATCH] fix(add helper for timestamp overflows) (#10330) * fix(add helper timestamp overflows) * fix(simplify code) * fix(make helper private) --- ethcore/src/error.rs | 3 ++ ethcore/src/verification/verification.rs | 35 +++++++++++++++++++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/ethcore/src/error.rs b/ethcore/src/error.rs index 2984dcdea..32cfe057a 100644 --- a/ethcore/src/error.rs +++ b/ethcore/src/error.rs @@ -89,6 +89,8 @@ pub enum BlockError { InvalidNumber(Mismatch), /// Block number isn't sensible. RidiculousNumber(OutOfBounds), + /// Timestamp header overflowed + TimestampOverflow, /// Too many transactions from a particular address. TooManyTransactions(Address), /// Parent given is unknown. @@ -138,6 +140,7 @@ impl fmt::Display for BlockError { UnknownParent(ref hash) => format!("Unknown parent: {}", hash), UnknownUncleParent(ref hash) => format!("Unknown uncle parent: {}", hash), UnknownEpochTransition(ref num) => format!("Unknown transition to epoch number: {}", num), + TimestampOverflow => format!("Timestamp overflow"), TooManyTransactions(ref address) => format!("Too many transactions from: {}", address), }; diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index 827b71439..3f5008a2b 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -40,6 +40,25 @@ use types::{BlockNumber, header::Header}; use types::transaction::SignedTransaction; use verification::queue::kind::blocks::Unverified; + +/// Returns `Ok` when the result less or equal to `i32::max_value` to prevent `SystemTime` to panic because +/// it is platform specific, may be i32 or i64. +/// +/// `Err Result { + let d1 = sys.duration_since(UNIX_EPOCH).map_err(|_| BlockError::TimestampOverflow)?; + let total_time = d1.checked_add(d2).ok_or(BlockError::TimestampOverflow)?; + + if total_time.as_secs() <= i32::max_value() as u64 { + Ok(sys + d2) + } else { + Err(BlockError::TimestampOverflow) + } +} + /// Preprocessed block data gathered in `verify_block_unordered` call pub struct PreverifiedBlock { /// Populated block header @@ -306,7 +325,7 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool, const ACCEPTABLE_DRIFT: Duration = Duration::from_secs(15); let max_time = SystemTime::now() + ACCEPTABLE_DRIFT; let invalid_threshold = max_time + ACCEPTABLE_DRIFT * 9; - let timestamp = UNIX_EPOCH + Duration::from_secs(header.timestamp()); + let timestamp = timestamp_checked_add(UNIX_EPOCH, Duration::from_secs(header.timestamp()))?; if timestamp > invalid_threshold { return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: Some(max_time), min: None, found: timestamp }))) @@ -328,8 +347,8 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result let gas_limit_divisor = engine.params().gas_limit_bound_divisor; if !engine.is_timestamp_valid(header.timestamp(), parent.timestamp()) { - let min = SystemTime::now() + Duration::from_secs(parent.timestamp() + 1); - let found = SystemTime::now() + Duration::from_secs(header.timestamp()); + let min = timestamp_checked_add(SystemTime::now(), Duration::from_secs(parent.timestamp().saturating_add(1)))?; + let found = timestamp_checked_add(SystemTime::now(), Duration::from_secs(header.timestamp()))?; return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: None, min: Some(min), found }))) } if header.number() != parent.number() + 1 { @@ -743,7 +762,8 @@ mod tests { check_fail_timestamp(family_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine, &bc), false); header = good.clone(); - header.set_timestamp(2450000000); + // will return `BlockError::TimestampOverflow` when timestamp > `i32::max_value()` + header.set_timestamp(i32::max_value() as u64); check_fail_timestamp(basic_test(&create_test_block_with_data(&header, &good_transactions, &good_uncles), engine), false); header = good.clone(); @@ -815,4 +835,11 @@ mod tests { check_fail(unordered_test(&create_test_block_with_data(&header, &bad_transactions, &[]), &engine), TooManyTransactions(keypair.address())); unordered_test(&create_test_block_with_data(&header, &good_transactions, &[]), &engine).unwrap(); } + + #[test] + fn checked_add_systime_dur() { + assert!(timestamp_checked_add(UNIX_EPOCH, Duration::new(i32::max_value() as u64 + 1, 0)).is_err()); + assert!(timestamp_checked_add(UNIX_EPOCH, Duration::new(i32::max_value() as u64, 0)).is_ok()); + assert!(timestamp_checked_add(UNIX_EPOCH, Duration::new(i32::max_value() as u64 - 1, 1_000_000_000)).is_ok()); + } }