From 425a96fc1c29ad3d02b15d10f67b1d139d38714a Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Tue, 18 Jun 2019 18:58:04 +0200 Subject: [PATCH] fix: aura don't add `SystemTime::now()` This commit does the following: - Prevent overflow in `verify_timestamp()` by not adding `now` to found faulty timestamp - Use explicit `CheckedSystemTime::checked_add` to prevent potential consensus issues because SystemTime is platform depedent - remove `#[cfg(not(time_checked_add))]` conditional compilation --- ethcore/src/engines/authority_round/mod.rs | 14 ++++++-------- ethcore/src/engines/clique/block_state.rs | 6 ++---- ethcore/src/engines/clique/mod.rs | 12 +++++------- ethcore/src/lib.rs | 5 +---- ethcore/src/verification/verification.rs | 7 +++---- 5 files changed, 17 insertions(+), 27 deletions(-) diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 6f2723bba71..11fab75ab9b 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -22,7 +22,7 @@ use std::iter::FromIterator; use std::ops::Deref; use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering as AtomicOrdering}; use std::sync::{Weak, Arc}; -use std::time::{UNIX_EPOCH, SystemTime, Duration}; +use std::time::{UNIX_EPOCH, Duration}; use block::*; use client::EngineClient; @@ -42,14 +42,12 @@ use itertools::{self, Itertools}; use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, Rlp}; use ethereum_types::{H256, H520, Address, U128, U256}; use parking_lot::{Mutex, RwLock}; +use time_utils::CheckedSystemTime; use types::BlockNumber; use types::header::{Header, ExtendedHeader}; use types::ancestry_action::AncestryAction; use unexpected::{Mismatch, OutOfBounds}; -#[cfg(not(time_checked_add))] -use time_utils::CheckedSystemTime; - mod finality; /// `AuthorityRound` params. @@ -584,10 +582,10 @@ fn verify_timestamp(step: &Step, header_step: u64) -> Result<(), BlockError> { // Returning it further won't recover the sync process. trace!(target: "engine", "verify_timestamp: block too early"); - let now = SystemTime::now(); - let found = now.checked_add(Duration::from_secs(oob.found)).ok_or(BlockError::TimestampOverflow)?; - let max = oob.max.and_then(|m| now.checked_add(Duration::from_secs(m))); - let min = oob.min.and_then(|m| now.checked_add(Duration::from_secs(m))); + let found = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(oob.found)) + .ok_or(BlockError::TimestampOverflow)?; + let max = oob.max.and_then(|m| CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(m))); + let min = oob.min.and_then(|m| CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(m))); let new_oob = OutOfBounds { min, max, found }; diff --git a/ethcore/src/engines/clique/block_state.rs b/ethcore/src/engines/clique/block_state.rs index 5e1c5f2fb46..913053eaa57 100644 --- a/ethcore/src/engines/clique/block_state.rs +++ b/ethcore/src/engines/clique/block_state.rs @@ -24,13 +24,11 @@ use engines::clique::{VoteType, DIFF_INTURN, DIFF_NOTURN, NULL_AUTHOR, SIGNING_D use error::{Error, BlockError}; use ethereum_types::{Address, H64}; use rand::Rng; +use time_utils::CheckedSystemTime; use types::BlockNumber; use types::header::Header; use unexpected::Mismatch; -#[cfg(not(feature = "time_checked_add"))] -use time_utils::CheckedSystemTime; - /// Type that keeps track of the state for a given vote // Votes that go against the proposal aren't counted since it's equivalent to not voting #[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] @@ -268,7 +266,7 @@ impl CliqueBlockState { // This is a quite bad API because we must mutate both variables even when already `inturn` fails // That's why we can't return early and must have the `if-else` in the end pub fn calc_next_timestamp(&mut self, timestamp: u64, period: u64) -> Result<(), Error> { - let inturn = UNIX_EPOCH.checked_add(Duration::from_secs(timestamp.saturating_add(period))); + let inturn = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(timestamp.saturating_add(period))); self.next_timestamp_inturn = inturn; diff --git a/ethcore/src/engines/clique/mod.rs b/ethcore/src/engines/clique/mod.rs index 052f0a578ce..15827477863 100644 --- a/ethcore/src/engines/clique/mod.rs +++ b/ethcore/src/engines/clique/mod.rs @@ -81,12 +81,10 @@ use parking_lot::RwLock; use rand::Rng; use super::signer::EngineSigner; use unexpected::{Mismatch, OutOfBounds}; +use time_utils::CheckedSystemTime; use types::BlockNumber; use types::header::{ExtendedHeader, Header}; -#[cfg(not(feature = "time_checked_add"))] -use time_utils::CheckedSystemTime; - use self::block_state::CliqueBlockState; use self::params::CliqueParams; @@ -545,7 +543,7 @@ impl Engine for Clique { // Don't waste time checking blocks from the future { - let limit = SystemTime::now().checked_add(Duration::from_secs(self.period)) + let limit = CheckedSystemTime::checked_add(SystemTime::now(), Duration::from_secs(self.period)) .ok_or(BlockError::TimestampOverflow)?; // This should succeed under the contraints that the system clock works @@ -555,7 +553,7 @@ impl Engine for Clique { let hdr = Duration::from_secs(header.timestamp()); if hdr > limit_as_dur { - let found = UNIX_EPOCH.checked_add(hdr).ok_or(BlockError::TimestampOverflow)?; + let found = CheckedSystemTime::checked_add(UNIX_EPOCH, hdr).ok_or(BlockError::TimestampOverflow)?; Err(BlockError::TemporarilyInvalid(OutOfBounds { min: None, @@ -666,8 +664,8 @@ impl Engine for Clique { // Ensure that the block's timestamp isn't too close to it's parent let limit = parent.timestamp().saturating_add(self.period); if limit > header.timestamp() { - let max = UNIX_EPOCH.checked_add(Duration::from_secs(header.timestamp())); - let found = UNIX_EPOCH.checked_add(Duration::from_secs(limit)) + let max = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(header.timestamp())); + let found = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(limit)) .ok_or(BlockError::TimestampOverflow)?; Err(BlockError::InvalidTimestamp(OutOfBounds { diff --git a/ethcore/src/lib.rs b/ethcore/src/lib.rs index a5f3adf5891..54010d2408f 100644 --- a/ethcore/src/lib.rs +++ b/ethcore/src/lib.rs @@ -15,7 +15,6 @@ // along with Parity Ethereum. If not, see . #![warn(missing_docs, unused_extern_crates)] -#![cfg_attr(feature = "time_checked_add", feature(time_checked_add))] //! Ethcore library //! @@ -101,6 +100,7 @@ extern crate rlp; extern crate rustc_hex; extern crate serde; extern crate stats; +extern crate time_utils; extern crate triehash_ethereum as triehash; extern crate unexpected; extern crate using_queue; @@ -151,9 +151,6 @@ extern crate fetch; #[cfg(all(test, feature = "price-info"))] extern crate parity_runtime; -#[cfg(not(time_checked_add))] -extern crate time_utils; - pub mod block; pub mod builtin; pub mod client; diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index fa458803cd5..94839051f76 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -40,7 +40,6 @@ use types::{BlockNumber, header::Header}; use types::transaction::SignedTransaction; use verification::queue::kind::blocks::Unverified; -#[cfg(not(time_checked_add))] use time_utils::CheckedSystemTime; /// Preprocessed block data gathered in `verify_block_unordered` call @@ -310,7 +309,7 @@ pub fn verify_header_params(header: &Header, engine: &dyn EthEngine, is_full: bo // this will resist overflow until `year 2037` let max_time = SystemTime::now() + ACCEPTABLE_DRIFT; let invalid_threshold = max_time + ACCEPTABLE_DRIFT * 9; - let timestamp = UNIX_EPOCH.checked_add(Duration::from_secs(header.timestamp())) + let timestamp = CheckedSystemTime::checked_add(UNIX_EPOCH, Duration::from_secs(header.timestamp())) .ok_or(BlockError::TimestampOverflow)?; if timestamp > invalid_threshold { @@ -334,9 +333,9 @@ fn verify_parent(header: &Header, parent: &Header, engine: &dyn EthEngine) -> Re if !engine.is_timestamp_valid(header.timestamp(), parent.timestamp()) { let now = SystemTime::now(); - let min = now.checked_add(Duration::from_secs(parent.timestamp().saturating_add(1))) + let min = CheckedSystemTime::checked_add(now, Duration::from_secs(parent.timestamp().saturating_add(1))) .ok_or(BlockError::TimestampOverflow)?; - let found = now.checked_add(Duration::from_secs(header.timestamp())) + let found = CheckedSystemTime::checked_add(now, Duration::from_secs(header.timestamp())) .ok_or(BlockError::TimestampOverflow)?; return Err(From::from(BlockError::InvalidTimestamp(OutOfBounds { max: None, min: Some(min), found }.into()))) }