From b70afbb37f8946a38ffdbf80fcfd112b814ff50f Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Mon, 22 May 2023 13:29:09 +0200 Subject: [PATCH] fix: ensure extradata is 32 bytes or less (#2775) --- bin/reth/src/args/network_args.rs | 4 +- bin/reth/src/args/payload_builder_args.rs | 65 +++++++++++++++++-- bin/reth/src/version.rs | 31 ++++++++- .../consensus/beacon/src/beacon_consensus.rs | 5 +- crates/primitives/src/constants.rs | 3 + .../rpc/rpc-types/src/eth/engine/payload.rs | 4 +- 6 files changed, 98 insertions(+), 14 deletions(-) diff --git a/bin/reth/src/args/network_args.rs b/bin/reth/src/args/network_args.rs index 94ed5d90e9..cdf0d483f4 100644 --- a/bin/reth/src/args/network_args.rs +++ b/bin/reth/src/args/network_args.rs @@ -1,6 +1,6 @@ //! clap [Args](clap::Args) for network related arguments. -use crate::version::P2P_VERSION; +use crate::version::P2P_CLIENT_VERSION; use clap::Args; use reth_net_nat::NatResolver; use reth_network::{HelloMessage, NetworkConfigBuilder}; @@ -38,7 +38,7 @@ pub struct NetworkArgs { pub peers_file: Option, /// Custom node identity - #[arg(long, value_name = "IDENTITY", default_value = P2P_VERSION)] + #[arg(long, value_name = "IDENTITY", default_value = P2P_CLIENT_VERSION)] pub identity: String, /// Secret key to use for this node. diff --git a/bin/reth/src/args/payload_builder_args.rs b/bin/reth/src/args/payload_builder_args.rs index caa7d94ef9..c56239dd74 100644 --- a/bin/reth/src/args/payload_builder_args.rs +++ b/bin/reth/src/args/payload_builder_args.rs @@ -1,12 +1,16 @@ -use crate::{utils::parse_duration_from_secs, version::P2P_VERSION}; -use clap::{builder::RangedU64ValueParser, Args}; -use std::time::Duration; +use crate::{utils::parse_duration_from_secs, version::default_extradata}; +use clap::{ + builder::{RangedU64ValueParser, TypedValueParser}, + Arg, Args, Command, +}; +use reth_primitives::constants::MAXIMUM_EXTRA_DATA_SIZE; +use std::{ffi::OsStr, time::Duration}; /// Parameters for configuring the Payload Builder #[derive(Debug, Args, PartialEq, Default)] pub struct PayloadBuilderArgs { - /// Extra block data set by the builder. - #[arg(long = "builder.extradata", help_heading = "Builder", default_value = P2P_VERSION)] + /// Block extra data set by the payload builder. + #[arg(long = "builder.extradata", help_heading = "Builder", value_parser=ExtradataValueParser::default(), default_value_t = default_extradata())] pub extradata: String, /// Target gas ceiling for built blocks. @@ -31,9 +35,35 @@ pub struct PayloadBuilderArgs { pub max_payload_tasks: usize, } +#[derive(Clone, Debug, Default)] +#[non_exhaustive] +struct ExtradataValueParser; + +impl TypedValueParser for ExtradataValueParser { + type Value = String; + + fn parse_ref( + &self, + _cmd: &Command, + _arg: Option<&Arg>, + value: &OsStr, + ) -> Result { + let val = + value.to_str().ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; + if val.as_bytes().len() > MAXIMUM_EXTRA_DATA_SIZE { + return Err(clap::Error::raw( + clap::error::ErrorKind::InvalidValue, + format!( + "Payload builder extradata size exceeds {MAXIMUM_EXTRA_DATA_SIZE}bytes limit" + ), + )) + } + Ok(val.to_string()) + } +} + #[cfg(test)] mod tests { - use super::*; use clap::{Args, Parser}; @@ -61,4 +91,27 @@ mod tests { ]) .is_err()); } + + #[test] + fn test_default_extradata() { + let extradata = default_extradata(); + let args = CommandParser::::parse_from([ + "reth", + "--builder.extradata", + extradata.as_str(), + ]) + .args; + assert_eq!(args.extradata, extradata); + } + + #[test] + fn test_invalid_extradata() { + let extradata = "x".repeat(MAXIMUM_EXTRA_DATA_SIZE + 1); + let args = CommandParser::::try_parse_from([ + "reth", + "--builder.extradata", + extradata.as_str(), + ]); + assert!(args.is_err()); + } } diff --git a/bin/reth/src/version.rs b/bin/reth/src/version.rs index 5a4bc16a22..17395d16ac 100644 --- a/bin/reth/src/version.rs +++ b/bin/reth/src/version.rs @@ -52,6 +52,33 @@ pub(crate) const LONG_VERSION: &str = concat!( /// ```text /// reth/v{major}.{minor}.{patch}/{target} /// ``` -#[allow(dead_code)] -pub(crate) const P2P_VERSION: &str = +pub(crate) const P2P_CLIENT_VERSION: &str = concat!("reth/v", env!("CARGO_PKG_VERSION"), "/", env!("VERGEN_CARGO_TARGET_TRIPLE")); + +/// The default extradata used for payload building. +/// +/// - The latest version from Cargo.toml +/// - The OS identifier +/// +/// # Example +/// +/// ```text +/// reth/v{major}.{minor}.{patch}/{OS} +/// ``` +pub fn default_extradata() -> String { + format!("reth/v{}/{}", env!("CARGO_PKG_VERSION"), std::env::consts::OS) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn assert_extradata_less_32bytes() { + let extradata = default_extradata(); + assert!( + extradata.as_bytes().len() <= 32, + "extradata must be less than 32 bytes: {extradata}" + ) + } +} diff --git a/crates/consensus/beacon/src/beacon_consensus.rs b/crates/consensus/beacon/src/beacon_consensus.rs index cbc1d4b90d..449d46845c 100644 --- a/crates/consensus/beacon/src/beacon_consensus.rs +++ b/crates/consensus/beacon/src/beacon_consensus.rs @@ -2,7 +2,8 @@ use reth_consensus_common::validation; use reth_interfaces::consensus::{Consensus, ConsensusError}; use reth_primitives::{ - Chain, ChainSpec, Hardfork, Header, SealedBlock, SealedHeader, EMPTY_OMMER_ROOT, U256, + constants::MAXIMUM_EXTRA_DATA_SIZE, Chain, ChainSpec, Hardfork, Header, SealedBlock, + SealedHeader, EMPTY_OMMER_ROOT, U256, }; use std::sync::Arc; @@ -89,7 +90,7 @@ impl Consensus for BeaconConsensus { /// From yellow paper: extraData: An arbitrary byte array containing data relevant to this block. /// This must be 32 bytes or fewer; formally Hx. fn validate_header_extradata(header: &Header) -> Result<(), ConsensusError> { - if header.extra_data.len() > 32 { + if header.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE { Err(ConsensusError::ExtraDataExceedsMax { len: header.extra_data.len() }) } else { Ok(()) diff --git a/crates/primitives/src/constants.rs b/crates/primitives/src/constants.rs index 225b59c0d2..e231aa57fc 100644 --- a/crates/primitives/src/constants.rs +++ b/crates/primitives/src/constants.rs @@ -10,6 +10,9 @@ pub const RETH_CLIENT_VERSION: &str = concat!("reth/v", env!("CARGO_PKG_VERSION" /// The first four bytes of the call data for a function call specifies the function to be called. pub const SELECTOR_LEN: usize = 4; +/// Maximum extra data size in a block after genesis +pub const MAXIMUM_EXTRA_DATA_SIZE: usize = 32; + /// The duration of a slot in seconds. /// /// This is the time period of 12 seconds in which a randomly chosen validator has time to propose a diff --git a/crates/rpc/rpc-types/src/eth/engine/payload.rs b/crates/rpc/rpc-types/src/eth/engine/payload.rs index d78f69e13b..358a511387 100644 --- a/crates/rpc/rpc-types/src/eth/engine/payload.rs +++ b/crates/rpc/rpc-types/src/eth/engine/payload.rs @@ -1,5 +1,5 @@ use reth_primitives::{ - constants::MIN_PROTOCOL_BASE_FEE_U256, + constants::{MAXIMUM_EXTRA_DATA_SIZE, MIN_PROTOCOL_BASE_FEE_U256}, proofs::{self, EMPTY_LIST_HASH}, Address, Block, Bloom, Bytes, Header, SealedBlock, TransactionSigned, UintTryTo, Withdrawal, H256, H64, U256, U64, @@ -126,7 +126,7 @@ impl TryFrom for SealedBlock { type Error = PayloadError; fn try_from(payload: ExecutionPayload) -> Result { - if payload.extra_data.len() > 32 { + if payload.extra_data.len() > MAXIMUM_EXTRA_DATA_SIZE { return Err(PayloadError::ExtraData(payload.extra_data)) }