fix: aura don't add SystemTime::now() (#10720)

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
This commit is contained in:
Niklas Adolfsson 2019-06-20 09:00:50 +02:00 committed by David
parent e1333ea1e9
commit 78d0a8696f
5 changed files with 17 additions and 27 deletions

View File

@ -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 };

View File

@ -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;

View File

@ -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<EthereumMachine> 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<EthereumMachine> 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<EthereumMachine> 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 {

View File

@ -15,7 +15,6 @@
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.
#![warn(missing_docs, unused_extern_crates)]
#![cfg_attr(feature = "time_checked_add", feature(time_checked_add))]
//! Ethcore library
//!
@ -103,6 +102,7 @@ extern crate parity_util_mem as malloc_size_of;
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;
@ -153,9 +153,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;

View File

@ -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
@ -303,7 +302,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 {
@ -327,9 +326,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())))
}