Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
fix: aura don't add SystemTime::now() (#10720)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
niklasad1 authored and s3krit committed Jun 25, 2019
1 parent 0cbf22b commit 2a5397f
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 27 deletions.
14 changes: 6 additions & 8 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -578,10 +576,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 };

Expand Down
6 changes: 2 additions & 4 deletions ethcore/src/engines/clique/block_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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;

Expand Down
12 changes: 5 additions & 7 deletions ethcore/src/engines/clique/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,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;
use self::step_service::StepService;
Expand Down Expand Up @@ -536,7 +534,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
Expand All @@ -546,7 +544,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,
Expand Down Expand Up @@ -657,8 +655,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 {
Expand Down
5 changes: 1 addition & 4 deletions ethcore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down Expand Up @@ -100,6 +99,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;
Expand Down Expand Up @@ -149,9 +149,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;
Expand Down
7 changes: 3 additions & 4 deletions ethcore/src/verification/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -310,7 +309,7 @@ pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool,
// 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 {
Expand All @@ -334,9 +333,9 @@ fn verify_parent(header: &Header, parent: &Header, engine: &EthEngine) -> Result

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 })))
}
Expand Down

0 comments on commit 2a5397f

Please sign in to comment.