CVE-2022-31111: Always-Incorrect Control Flow Implementation (CWE-670)
value: Frontier is Substrate's Ethereum compatibility layer. In affected versions the truncation done when converting between EVM balance type and Substrate balance type was incorrectly implemented. This leads to possible discrepancy between appeared EVM transfer value and actual Substrate value transferred. It is recommended that an emergency upgrade to be planned and EVM execution temporarily paused in the mean time. The issue is patched in Frontier master branch commit fed5e0a9577c10bea021721e8c2c5c378e16bf66 and polkadot-v0.9.22 branch commit e3e427fa2e5d1200a784679f8015d4774cedc934. This vulnerability affects only EVM internal states, but not Substrate balance states or node. You can temporarily pause EVM execution (by setting up a Substrate `CallFilter` that disables `pallet-evm` and `pallet-ethereum` calls before the patch can be applied.
CWE description: The code contains a control flow path that does not reflect the algorithm that the path is intended to implement, leading to incorrect behavior any time this path is navigated.
CVSS v3 base score: 5.3
Published: 2022-07-06T18:15Z
Repository: https://github.com/paritytech/frontier
Language: Rust
Files changed: frame/ethereum/src/lib.rs, frame/evm/src/lib.rs, frame/evm/src/runner/stack.rs, frame/evm/src/tests.rs, primitives/evm/src/validation.rs, template/runtime/src/lib.rs
Commit date: 2022-06-29 11:28:33 +0200
Commit message: Limit number of iterations in genesis nonce building (#753)


Diff: diff --git a/frame/ethereum/src/lib.rs b/frame/ethereum/src/lib.rs
index 6984991a..a5754157 100644
--- a/frame/ethereum/src/lib.rs
+++ b/frame/ethereum/src/lib.rs
@@ -685,11 +685,11 @@ impl<T: Config> Pallet<T> {
 				let res = match T::Runner::call(
 					from,
 					target,
 					input,
 					value,
-					gas_limit.low_u64(),
+					gas_limit.unique_saturated_into(),
 					max_fee_per_gas,
 					max_priority_fee_per_gas,
 					nonce,
 					access_list,
 					is_transactional,
@@ -713,11 +713,11 @@ impl<T: Config> Pallet<T> {
 			ethereum::TransactionAction::Create => {
 				let res = match T::Runner::create(
 					from,
 					input,
 					value,
-					gas_limit.low_u64(),
+					gas_limit.unique_saturated_into(),
 					max_fee_per_gas,
 					max_priority_fee_per_gas,
 					nonce,
 					access_list,
 					is_transactional,
diff --git a/frame/evm/src/lib.rs b/frame/evm/src/lib.rs
index ae68ae86..18e9040a 100644
--- a/frame/evm/src/lib.rs
+++ b/frame/evm/src/lib.rs
@@ -75,11 +75,11 @@ use frame_system::RawOrigin;
 use sp_core::{Hasher, H160, H256, U256};
 use sp_runtime::{
 	traits::{BadOrigin, Saturating, UniqueSaturatedInto, Zero},
 	AccountId32, DispatchErrorWithPostInfo,
 };
-use sp_std::vec::Vec;
+use sp_std::{cmp::min, vec::Vec};
 
 pub use evm::{
 	Config as EvmConfig, Context, ExitError, ExitFatal, ExitReason, ExitRevert, ExitSucceed,
 };
 #[cfg(feature = "std")]
@@ -436,25 +436,30 @@ pub mod pallet {
 	pub struct GenesisConfig {
 		pub accounts: std::collections::BTreeMap<H160, GenesisAccount>,
 	}
 
 	#[pallet::genesis_build]
-	impl<T: Config> GenesisBuild<T> for GenesisConfig {
+	impl<T: Config> GenesisBuild<T> for GenesisConfig
+	where
+		U256: UniqueSaturatedInto<BalanceOf<T>>,
+	{
 		fn build(&self) {
+			const MAX_ACCOUNT_NONCE: usize = 100;
+
 			for (address, account) in &self.accounts {
 				let account_id = T::AddressMapping::into_account_id(*address);
 
 				// ASSUME: in one single EVM transaction, the nonce will not increase more than
 				// `u128::max_value()`.
-				for _ in 0..account.nonce.low_u128() {
+				for _ in 0..min(
+					MAX_ACCOUNT_NONCE,
+					UniqueSaturatedInto::<usize>::unique_saturated_into(account.nonce),
+				) {
 					frame_system::Pallet::<T>::inc_account_nonce(&account_id);
 				}
 
-				T::Currency::deposit_creating(
-					&account_id,
-					account.balance.low_u128().unique_saturated_into(),
-				);
+				T::Currency::deposit_creating(&account_id, account.balance.unique_saturated_into());
 
 				Pallet::<T>::create_account(*address, account.code.clone());
 
 				for (index, value) in &account.storage {
 					<AccountStorages<T>>::insert(address, index, value);
@@ -733,10 +738,11 @@ where
 	C::NegativeImbalance: Imbalance<
 		<C as Currency<<T as frame_system::Config>::AccountId>>::Balance,
 		Opposite = C::PositiveImbalance,
 	>,
 	OU: OnUnbalanced<NegativeImbalanceOf<C, T>>,
+	U256: UniqueSaturatedInto<<C as Currency<<T as frame_system::Config>::AccountId>>::Balance>,
 {
 	// Kept type as Option to satisfy bound of Default
 	type LiquidityInfo = Option<NegativeImbalanceOf<C, T>>;
 
 	fn withdraw_fee(who: &H160, fee: U256) -> Result<Self::LiquidityInfo, Error<T>> {
@@ -744,11 +750,11 @@ where
 			return Ok(None);
 		}
 		let account_id = T::AddressMapping::into_account_id(*who);
 		let imbalance = C::withdraw(
 			&account_id,
-			fee.low_u128().unique_saturated_into(),
+			fee.unique_saturated_into(),
 			WithdrawReasons::FEE,
 			ExistenceRequirement::AllowDeath,
 		)
 		.map_err(|_| Error::<T>::BalanceLow)?;
 		Ok(Some(imbalance))
@@ -764,11 +770,11 @@ where
 			let account_id = T::AddressMapping::into_account_id(*who);
 
 			// Calculate how much refund we should return
 			let refund_amount = paid
 				.peek()
-				.saturating_sub(corrected_fee.low_u128().unique_saturated_into());
+				.saturating_sub(corrected_fee.unique_saturated_into());
 			// refund to the account that paid the fees. If this fails, the
 			// account might have dropped below the existential balance. In
 			// that case we don't refund anything.
 			let refund_imbalance = C::deposit_into_existing(&account_id, refund_amount)
 				.unwrap_or_else(|_| C::PositiveImbalance::zero());
@@ -795,11 +801,11 @@ where
 			let adjusted_paid = paid
 				.offset(refund_imbalance)
 				.same()
 				.unwrap_or_else(|_| C::NegativeImbalance::zero());
 
-			let (base_fee, tip) = adjusted_paid.split(base_fee.low_u128().unique_saturated_into());
+			let (base_fee, tip) = adjusted_paid.split(base_fee.unique_saturated_into());
 			// Handle base fee. Can be either burned, rationed, etc ...
 			OU::on_unbalanced(base_fee);
 			return Some(tip);
 		}
 		None
@@ -819,11 +825,14 @@ impl<T> OnChargeEVMTransaction<T> for ()
 	where
 	T: Config,
 	<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::PositiveImbalance:
 		Imbalance<<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance, Opposite = <T::Currency as Currency<<T as frame_system::Config>::AccountId>>::NegativeImbalance>,
 	<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::NegativeImbalance:
-		Imbalance<<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance, Opposite = <T::Currency as Currency<<T as frame_system::Config>::AccountId>>::PositiveImbalance>, {
+Imbalance<<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance, Opposite = <T::Currency as Currency<<T as frame_system::Config>::AccountId>>::PositiveImbalance>,
+U256: UniqueSaturatedInto<BalanceOf<T>>,
+
+{
 	// Kept type as Option to satisfy bound of Default
 	type LiquidityInfo = Option<NegativeImbalanceOf<T::Currency, T>>;
 
 	fn withdraw_fee(
 		who: &H160,
diff --git a/frame/evm/src/runner/stack.rs b/frame/evm/src/runner/stack.rs
index 4dedfbd9..317025af 100644
--- a/frame/evm/src/runner/stack.rs
+++ b/frame/evm/src/runner/stack.rs
@@ -16,12 +16,13 @@
 // limitations under the License.
 
 //! EVM stack-based runner.
 
 use crate::{
-	runner::Runner as RunnerT, AccountCodes, AccountStorages, AddressMapping, BlockHashMapping,
-	Config, Error, Event, FeeCalculator, OnChargeEVMTransaction, Pallet, RunnerError,
+	runner::Runner as RunnerT, AccountCodes, AccountStorages, AddressMapping, BalanceOf,
+	BlockHashMapping, Config, Error, Event, FeeCalculator, OnChargeEVMTransaction, Pallet,
+	RunnerError,
 };
 use evm::{
 	backend::Backend as BackendT,
 	executor::stack::{Accessed, StackExecutor, StackState as StackStateT, StackSubstateMetadata},
 	ExitError, ExitReason, Transfer,
@@ -35,11 +36,14 @@ use sp_std::{boxed::Box, collections::btree_set::BTreeSet, marker::PhantomData,
 #[derive(Default)]
 pub struct Runner<T: Config> {
 	_marker: PhantomData<T>,
 }
 
-impl<T: Config> Runner<T> {
+impl<T: Config> Runner<T>
+where
+	BalanceOf<T>: TryFrom<U256> + Into<U256>,
+{
 	/// Execute an already validated EVM operation.
 	fn execute<'config, 'precompiles, F, R>(
 		source: H160,
 		value: U256,
 		gas_limit: u64,
@@ -190,11 +194,14 @@ impl<T: Config> Runner<T> {
 			logs: state.substate.logs,
 		})
 	}
 }
 
-impl<T: Config> RunnerT<T> for Runner<T> {
+impl<T: Config> RunnerT<T> for Runner<T>
+where
+	BalanceOf<T>: TryFrom<U256> + Into<U256>,
+{
 	type Error = Error<T>;
 
 	fn validate(
 		source: H160,
 		target: Option<H160>,
@@ -577,10 +584,12 @@ impl<'vicinity, 'config, T: Config> BackendT for SubstrateStackState<'vicinity,
 	}
 }
 
 impl<'vicinity, 'config, T: Config> StackStateT<'config>
 	for SubstrateStackState<'vicinity, 'config, T>
+where
+	BalanceOf<T>: TryFrom<U256> + Into<U256>,
 {
 	fn metadata(&self) -> &StackSubstateMetadata<'config> {
 		self.substate.metadata()
 	}
 
@@ -665,11 +674,14 @@ impl<'vicinity, 'config, T: Config> StackStateT<'config>
 		let target = T::AddressMapping::into_account_id(transfer.target);
 
 		T::Currency::transfer(
 			&source,
 			&target,
-			transfer.value.low_u128().unique_saturated_into(),
+			transfer
+				.value
+				.try_into()
+				.map_err(|_| ExitError::OutOfFund)?,
 			ExistenceRequirement::AllowDeath,
 		)
 		.map_err(|_| ExitError::OutOfFund)
 	}
 
diff --git a/frame/evm/src/tests.rs b/frame/evm/src/tests.rs
index 2070564a..571cbe3b 100644
--- a/frame/evm/src/tests.rs
+++ b/frame/evm/src/tests.rs
@@ -266,12 +266,14 @@ fn issuance_after_tip() {
 			Vec::new(),
 		);
 		result.expect("EVM can be called");
 		let after_tip = <Test as Config>::Currency::total_issuance();
 		// Only base fee is burned
-		let (base_fee, _) = <Test as Config>::FeeCalculator::min_gas_price();
-		assert_eq!(after_tip, (before_tip - (base_fee.low_u64() * 21_000)));
+		let base_fee: u64 = <Test as Config>::FeeCalculator::min_gas_price()
+			.0
+			.unique_saturated_into();
+		assert_eq!(after_tip, (before_tip - (base_fee * 21_000)));
 	});
 }
 
 #[test]
 fn author_same_balance_without_tip() {
@@ -353,11 +355,11 @@ fn refunds_and_priority_should_work() {
 		let after_call = EVM::account_basic(&H160::default()).0.balance;
 		// The tip is deducted but never refunded to the caller.
 		assert_eq!(after_call, before_call - total_cost);
 
 		let after_tip = EVM::account_basic(&author).0.balance;
-		assert_eq!(after_tip, (before_tip + actual_tip.low_u128()));
+		assert_eq!(after_tip, (before_tip + actual_tip));
 	});
 }
 
 #[test]
 fn call_should_fail_with_priority_greater_than_max_fee() {
diff --git a/primitives/evm/src/validation.rs b/primitives/evm/src/validation.rs
index c5c62e0f..031cf824 100644
--- a/primitives/evm/src/validation.rs
+++ b/primitives/evm/src/validation.rs
@@ -15,10 +15,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 #![allow(clippy::comparison_chain)]
 
 pub use evm::backend::Basic as Account;
+use frame_support::sp_runtime::traits::UniqueSaturatedInto;
 use sp_core::{H160, H256, U256};
 use sp_std::vec::Vec;
 
 #[derive(Debug)]
 pub struct CheckEvmTransactionInput {
@@ -179,11 +180,11 @@ impl<'config, E: From<InvalidEvmTransactionError>> CheckEvmTransaction<'config,
 
 	fn validate_common(&self) -> Result<&Self, E> {
 		// We must ensure a transaction can pay the cost of its data bytes.
 		// If it can't it should not be included in a block.
 		let mut gasometer = evm::gasometer::Gasometer::new(
-			self.transaction.gas_limit.low_u64(),
+			self.transaction.gas_limit.unique_saturated_into(),
 			self.config.evm_config,
 		);
 		let transaction_cost = if self.transaction.to.is_some() {
 			evm::gasometer::call_transaction_cost(
 				&self.transaction.input,
diff --git a/template/runtime/src/lib.rs b/template/runtime/src/lib.rs
index d81b2628..83c4f265 100644
--- a/template/runtime/src/lib.rs
+++ b/template/runtime/src/lib.rs
@@ -22,11 +22,11 @@ use sp_core::{
 };
 use sp_runtime::{
 	create_runtime_str, generic, impl_opaque_keys,
 	traits::{
 		AccountIdLookup, BlakeTwo256, Block as BlockT, DispatchInfoOf, Dispatchable,
-		IdentifyAccount, NumberFor, PostDispatchInfoOf, Verify,
+		IdentifyAccount, NumberFor, PostDispatchInfoOf, UniqueSaturatedInto, Verify,
 	},
 	transaction_validity::{TransactionSource, TransactionValidity, TransactionValidityError},
 	ApplyExtrinsicResult, MultiSignature,
 };
 use sp_std::{marker::PhantomData, prelude::*};
@@ -662,11 +662,11 @@ impl_runtime_apis! {
 			<Runtime as pallet_evm::Config>::Runner::call(
 				from,
 				to,
 				data,
 				value,
-				gas_limit.low_u64(),
+				gas_limit.unique_saturated_into(),
 				max_fee_per_gas,
 				max_priority_fee_per_gas,
 				nonce,
 				access_list.unwrap_or_default(),
 				is_transactional,
@@ -699,11 +699,11 @@ impl_runtime_apis! {
 			let evm_config = config.as_ref().unwrap_or(<Runtime as pallet_evm::Config>::config());
 			<Runtime as pallet_evm::Config>::Runner::create(
 				from,
 				data,
 				value,
-				gas_limit.low_u64(),
+				gas_limit.unique_saturated_into(),
 				max_fee_per_gas,
 				max_priority_fee_per_gas,
 				nonce,
 				access_list.unwrap_or_default(),
 				is_transactional,

Vulnerable code: 					gas_limit.low_u64(),
					gas_limit.low_u64(),
use sp_std::vec::Vec;
	impl<T: Config> GenesisBuild<T> for GenesisConfig {
				for _ in 0..account.nonce.low_u128() {
				T::Currency::deposit_creating(
					&account_id,
					account.balance.low_u128().unique_saturated_into(),
				);
			fee.low_u128().unique_saturated_into(),
				.saturating_sub(corrected_fee.low_u128().unique_saturated_into());
			let (base_fee, tip) = adjusted_paid.split(base_fee.low_u128().unique_saturated_into());
		Imbalance<<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance, Opposite = <T::Currency as Currency<<T as frame_system::Config>::AccountId>>::PositiveImbalance>, {
	runner::Runner as RunnerT, AccountCodes, AccountStorages, AddressMapping, BlockHashMapping,
	Config, Error, Event, FeeCalculator, OnChargeEVMTransaction, Pallet, RunnerError,
impl<T: Config> Runner<T> {
impl<T: Config> RunnerT<T> for Runner<T> {
			transfer.value.low_u128().unique_saturated_into(),
		let (base_fee, _) = <Test as Config>::FeeCalculator::min_gas_price();
		assert_eq!(after_tip, (before_tip - (base_fee.low_u64() * 21_000)));
		assert_eq!(after_tip, (before_tip + actual_tip.low_u128()));
			self.transaction.gas_limit.low_u64(),
		IdentifyAccount, NumberFor, PostDispatchInfoOf, Verify,
				gas_limit.low_u64(),
				gas_limit.low_u64(),
Fixed code: 					gas_limit.unique_saturated_into(),
					gas_limit.unique_saturated_into(),
use sp_std::{cmp::min, vec::Vec};
	impl<T: Config> GenesisBuild<T> for GenesisConfig
	where
		U256: UniqueSaturatedInto<BalanceOf<T>>,
	{
			const MAX_ACCOUNT_NONCE: usize = 100;

				for _ in 0..min(
					MAX_ACCOUNT_NONCE,
					UniqueSaturatedInto::<usize>::unique_saturated_into(account.nonce),
				) {
				T::Currency::deposit_creating(&account_id, account.balance.unique_saturated_into());
	U256: UniqueSaturatedInto<<C as Currency<<T as frame_system::Config>::AccountId>>::Balance>,
			fee.unique_saturated_into(),
				.saturating_sub(corrected_fee.unique_saturated_into());
			let (base_fee, tip) = adjusted_paid.split(base_fee.unique_saturated_into());
Imbalance<<T::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance, Opposite = <T::Currency as Currency<<T as frame_system::Config>::AccountId>>::PositiveImbalance>,
U256: UniqueSaturatedInto<BalanceOf<T>>,

{
	runner::Runner as RunnerT, AccountCodes, AccountStorages, AddressMapping, BalanceOf,
	BlockHashMapping, Config, Error, Event, FeeCalculator, OnChargeEVMTransaction, Pallet,
	RunnerError,
impl<T: Config> Runner<T>
where
	BalanceOf<T>: TryFrom<U256> + Into<U256>,
{
impl<T: Config> RunnerT<T> for Runner<T>
where
	BalanceOf<T>: TryFrom<U256> + Into<U256>,
{
where
	BalanceOf<T>: TryFrom<U256> + Into<U256>,
			transfer
				.value
				.try_into()
				.map_err(|_| ExitError::OutOfFund)?,
		let base_fee: u64 = <Test as Config>::FeeCalculator::min_gas_price()
			.0
			.unique_saturated_into();
		assert_eq!(after_tip, (before_tip - (base_fee * 21_000)));
		assert_eq!(after_tip, (before_tip + actual_tip));
use frame_support::sp_runtime::traits::UniqueSaturatedInto;
			self.transaction.gas_limit.unique_saturated_into(),
		IdentifyAccount, NumberFor, PostDispatchInfoOf, UniqueSaturatedInto, Verify,
				gas_limit.unique_saturated_into(),
				gas_limit.unique_saturated_into(),