fix: harden fee-path panic sites against malicious transactions

pallets/dnt-fee-controller/src/lib.rs

@@ -18,6 +18,8 @@
 // information.
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// expect_used is not denied because FRAME macros expand to expect() internally.
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 
 pub use pallet::*;
 
@@ -55,13 +57,25 @@ pub mod pallet {
 		ArithmeticError,
 	}
 
+	#[pallet::type_value]
+	pub fn DefaultFeeVaultPrecompileAddress<T: Config>() -> H160 {
+		H160::from_low_u64_be(0x807)
+	}
+
+	#[pallet::type_value]
+	pub fn DefaultValidatorPercentage<T: Config>() -> U256 {
+		U256::from(50)
+	}
+
 	#[pallet::storage]
 	#[pallet::getter(fn fee_vault_precompile_address)]
-	pub type FeeVaultPrecompileAddressStorage<T: Config> = StorageValue<_, H160, OptionQuery>;
+	pub type FeeVaultPrecompileAddressStorage<T: Config> =
+		StorageValue<_, H160, ValueQuery, DefaultFeeVaultPrecompileAddress<T>>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn validator_percentage)]
-	pub type ValidatorPercentageStorage<T: Config> = StorageValue<_, U256, OptionQuery>;
+	pub type ValidatorPercentageStorage<T: Config> =
+		StorageValue<_, U256, ValueQuery, DefaultValidatorPercentage<T>>;
 
 	#[pallet::config]
 	pub trait Config:
@@ -83,10 +97,7 @@ pub mod pallet {
 	impl<T: Config> Default for GenesisConfig<T> {
 		fn default() -> Self {
 			Self {
-				fee_vault_precompile_address: <H160 as core::str::FromStr>::from_str(
-					"0x0000000000000000000000000000000000000807",
-				)
-				.unwrap(),
+				fee_vault_precompile_address: H160::from_low_u64_be(0x807),
 				validator_percentage: 50.into(),
 				_config: Default::default(),
 			}
@@ -117,7 +128,7 @@ pub mod pallet {
 		}
 
 		fn get_fee_vault() -> H160 {
-			Self::fee_vault_precompile_address().unwrap()
+			Self::fee_vault_precompile_address()
 		}
 
 		fn withdraw_fee(
@@ -133,7 +144,7 @@ pub mod pallet {
 				return Err(Error::<T>::InvalidConversionRate);
 			}
 
-			let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get().unwrap();
+			let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get();
 			let mapped_amount = amount
 				.checked_mul(conversion_rate.0)
 				.map(|v| v.div_mod(conversion_rate.1).0);
@@ -178,7 +189,7 @@ pub mod pallet {
 				_ => return Err(Error::ArithmeticError),
 			};
 
-			let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get().unwrap();
+			let fee_vault = FeeVaultPrecompileAddressStorage::<T>::get();
 			T::ERC20Manager::withdraw_amount(token, fee_vault, mapped_amount)
 				.map_err(|_| Error::<T>::ERC20WithdrawFailed)?;
 			T::ERC20Manager::deposit_amount(token, from, mapped_amount)
@@ -212,7 +223,7 @@ pub mod pallet {
 			};
 
 			let validator_share = match to {
-				Some(_) => ValidatorPercentageStorage::<T>::get().unwrap(),
+				Some(_) => ValidatorPercentageStorage::<T>::get(),
 				_ => 100.into(),
 			};
 
@@ -233,13 +244,9 @@ pub mod pallet {
 			)
 			.map_err(|_| Error::<T>::FeeVaultOverflow)?;
 
-			if to.is_some() {
-				pallet_fee_rewards_vault::Pallet::<T>::add_claimable_reward(
-					to.unwrap(),
-					token,
-					dapp_fee,
-				)
-				.map_err(|_| Error::<T>::FeeVaultOverflow)?;
+			if let Some(dapp) = to {
+				pallet_fee_rewards_vault::Pallet::<T>::add_claimable_reward(dapp, token, dapp_fee)
+					.map_err(|_| Error::<T>::FeeVaultOverflow)?;
 			}
 
 			Ok((validator_fee, dapp_fee))
@@ -256,7 +263,7 @@ pub mod pallet {
 		}
 
 		pub fn get_validator_percentage() -> U256 {
-			ValidatorPercentageStorage::<T>::get().unwrap()
+			ValidatorPercentageStorage::<T>::get()
 		}
 	}
 }

pallets/dnt-fee-controller/src/tests.rs

@@ -27,6 +27,7 @@ use crate::mock::{
 use super::*;
 use mock::{new_test_ext, MeaninglessAddress, MeaninglessAddress2, Test};
 use runner::OnChargeDecentralizedNativeTokenFee;
+use sp_core::H160;
 
 #[test]
 fn withdraw_fee_calls_deposit_and_withdraw() {
@@ -125,7 +126,7 @@ fn pay_fees_calls_vault_pallet() {
 	new_test_ext().execute_with(|| {
 		let meaningless_amount = 100.into();
 		let validator_amount =
-			DNTFeeController::validator_percentage().unwrap() * meaningless_amount / 100;
+			DNTFeeController::validator_percentage() * meaningless_amount / 100;
 		let result = <Pallet<Test> as OnChargeDecentralizedNativeTokenFee>::pay_fees(
 			MeaninglessTokenAddress::get(),
 			(1.into(), 1.into()),
@@ -274,10 +275,50 @@ fn fails_withdraw_if_erc20_manager_fails() {
 #[test]
 fn validator_percentage_updates() {
 	new_test_ext().execute_with(|| {
-		assert_eq!(DNTFeeController::validator_percentage().unwrap(), 50.into());
+		assert_eq!(DNTFeeController::validator_percentage(), 50.into());
 
 		assert!(DNTFeeController::set_validator_percentage(10.into()).is_ok());
 
-		assert_eq!(DNTFeeController::validator_percentage().unwrap(), 10.into());
+		assert_eq!(DNTFeeController::validator_percentage(), 10.into());
+	});
+}
+
+#[test]
+fn missing_storage_falls_back_to_defaults_without_panicking() {
+	new_test_ext().execute_with(|| {
+		FeeVaultPrecompileAddressStorage::<Test>::kill();
+		ValidatorPercentageStorage::<Test>::kill();
+
+		assert_eq!(
+			<Pallet<Test> as OnChargeDecentralizedNativeTokenFee>::get_fee_vault(),
+			H160::from_low_u64_be(0x807)
+		);
+		assert_eq!(DNTFeeController::get_validator_percentage(), 50.into());
+
+		let result = <Pallet<Test> as OnChargeDecentralizedNativeTokenFee>::withdraw_fee(
+			MeaninglessAddress::get(),
+			MeaninglessTokenAddress::get(),
+			(1.into(), 1.into()),
+			100.into(),
+		);
+		assert!(result.is_ok());
+
+		let result = <Pallet<Test> as OnChargeDecentralizedNativeTokenFee>::correct_fee(
+			MeaninglessAddress::get(),
+			MeaninglessTokenAddress::get(),
+			(1.into(), 1.into()),
+			100.into(),
+			10.into(),
+		);
+		assert!(result.is_ok());
+
+		let result = <Pallet<Test> as OnChargeDecentralizedNativeTokenFee>::pay_fees(
+			MeaninglessTokenAddress::get(),
+			(1.into(), 1.into()),
+			100.into(),
+			MeaninglessAddress::get(),
+			Some(MeaninglessAddress2::get()),
+		);
+		assert_eq!(result.unwrap(), (50.into(), 50.into()));
 	});
 }

pallets/sponsored-transactions/src/lib.rs

@@ -18,6 +18,8 @@
 // information.
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// expect_used is not denied because FRAME macros expand to expect() internally.
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 
 use sp_core::H160;
 
@@ -185,7 +187,15 @@ pub mod pallet {
 			let dispatch = pallet_ethereum::Pallet::<T>::transact(origin, transaction)
 				.map_err(|_| DispatchError::Other("Signature doesn't meet with sponsor address"))?;
 
-			let gas_used = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap())
+			// Fall back to the declared gas limit (worst case) if the dispatch
+			// reports no actual weight.
+			let transaction_weight = dispatch.actual_weight.unwrap_or_else(|| {
+				<T as pallet_evm::Config>::GasWeightMapping::gas_to_weight(
+					gas_limit.unique_saturated_into(),
+					true,
+				)
+			});
+			let gas_used = Self::gas_from_actual_weight(transaction_weight)
 				.map_err(|_| DispatchError::Other("Arithmetic error due to overflow."))?;
 
 			let gas_left = match gas_limit.checked_sub(gas_used.into()) {
@@ -395,8 +405,12 @@ pub mod pallet {
 		}
 
 		fn get_meta_trx_signer(signature: Vec<u8>, message: H256) -> Option<H160> {
+			let signature: [u8; 65] = match signature.as_slice().try_into() {
+				Ok(signature) => signature,
+				Err(_) => return None,
+			};
 			let result = match sp_io::crypto::secp256k1_ecdsa_recover(
-				signature.as_slice().try_into().unwrap(),
+				&signature,
 				message.as_fixed_bytes(),
 			) {
 				Ok(pubkey) => {

pallets/sponsored-transactions/src/tests.rs

@@ -214,3 +214,28 @@ fn check_correct_fee_management(called_arguments: Vec<(bool, H160, H160, U256)>)
 	}
 	assert_eq!(total_deposited, total_withdrawn);
 }
+
+#[test]
+fn fail_to_execute_meta_transaction_with_short_meta_signature() {
+	new_test_ext().execute_with(|| {
+		let trx1 = get_transaction_from_bytes(RawTransaction0::get());
+
+		let from = recover_signer(&trx1).unwrap();
+		let origin: <Runtime as frame_system::Config>::RuntimeOrigin =
+			pallet_ethereum::Origin::EthereumTransaction(from).into();
+
+		// A signature shorter than 65 bytes must be rejected, not panic.
+		let error = crate::Pallet::<Runtime>::send_sponsored_transaction(
+			origin.clone(),
+			trx1.clone(),
+			Sponsor::get(),
+			vec![0u8; 10],
+		)
+		.unwrap_err();
+
+		assert!(matches!(
+			error,
+			DispatchError::Other("Invalid metatransaction signature")
+		));
+	});
+}

pallets/token-fee-controller/validator-fee-selector/src/lib.rs

@@ -18,6 +18,8 @@
 // information.
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// expect_used is not denied because FRAME macros expand to expect() internally.
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 
 pub use pallet::*;
 use sp_core::{H160, U256};
@@ -72,7 +74,7 @@ pub mod pallet {
 
 	#[pallet::storage]
 	#[pallet::getter(fn default_controller)]
-	pub type DefaultController<T: Config> = StorageValue<_, H160, OptionQuery>;
+	pub type DefaultController<T: Config> = StorageValue<_, H160, ValueQuery>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn conversion_rate_fee_tokens)]
@@ -154,7 +156,7 @@ pub mod pallet {
 
 		fn conversion_rate_controller(validator: H160) -> H160 {
 			ValidatorConversionRateController::<T>::get(validator)
-				.unwrap_or(DefaultController::<T>::get().unwrap())
+				.unwrap_or_else(DefaultController::<T>::get)
 		}
 
 		fn conversion_rate(sender: H160, validator: H160, token: H160) -> (U256, U256) {

pallets/token-fee-controller/validator-fee-selector/src/tests.rs

@@ -139,4 +139,28 @@ fn not_supported_by_validator_if_not_supported_by_chain() {
             token,
         ), false);
     });
-}
+}
+#[test]
+fn missing_default_controller_does_not_panic() {
+    ExtBuilder::default().build().execute_with(|| {
+        crate::DefaultController::<Runtime>::kill();
+
+        assert_eq!(
+            <ValidatorFeeSelector as crate::ValidatorFeeTokenController>::conversion_rate_controller(
+                MeaninglessAccount::get()
+            ),
+            H160::zero()
+        );
+
+        // With a zero-address controller the simulator call yields no usable
+        // output, so the conversion rate falls back to (1, 1).
+        assert_eq!(
+            <ValidatorFeeSelector as crate::ValidatorFeeTokenController>::conversion_rate(
+                MeaninglessAccount::get(),
+                MeaninglessAccount::get(),
+                MeaninglessTokenAddress::get(),
+            ),
+            (1.into(), 1.into())
+        );
+    });
+}

pallets/zero-gas-transactions/src/lib.rs

@@ -18,6 +18,8 @@
 // information.
 
 #![cfg_attr(not(feature = "std"), no_std)]
+// expect_used is not denied because FRAME macros expand to expect() internally.
+#![cfg_attr(not(test), deny(clippy::unwrap_used))]
 
 use log;
 use pallet_evm::TransactionValidationError;
@@ -177,13 +179,24 @@ pub mod pallet {
 			let origin: T::RuntimeOrigin =
 				pallet_ethereum::Origin::EthereumTransaction(from).into();
 
+			let transaction_data: TransactionData = (&transaction).into();
+			let transaction_gas_limit = transaction_data.gas_limit;
+
 			let dispatch =
 				pallet_ethereum::Pallet::<T>::transact(origin, transaction).map_err(|e| {
 					log::debug!(target: LOG_TARGET, "Dispatch transaction error: {:?}", e);
 					DispatchError::Other("Signature doesn't meet with sponsor address")
 				})?;
 
-			let used_gas = Self::gas_from_actual_weight(dispatch.actual_weight.unwrap())
+			// Fall back to the declared gas limit (worst case) if the dispatch
+			// reports no actual weight.
+			let transaction_weight = dispatch.actual_weight.unwrap_or_else(|| {
+				T::GasWeightMapping::gas_to_weight(
+					transaction_gas_limit.unique_saturated_into(),
+					true,
+				)
+			});
+			let used_gas = Self::gas_from_actual_weight(transaction_weight)
 				.map_err(|_| DispatchError::Other("Arithmetic error due to overflows"))?;
 
 			Ok(frame_support::dispatch::PostDispatchInfo {
@@ -319,8 +332,12 @@ pub mod pallet {
 		}
 
 		fn get_zero_gas_trx_signer(signature: Vec<u8>, message: H256) -> Option<H160> {
+			let signature: [u8; 65] = match signature.as_slice().try_into() {
+				Ok(signature) => signature,
+				Err(_) => return None,
+			};
 			let result = match sp_io::crypto::secp256k1_ecdsa_recover(
-				signature.as_slice().try_into().unwrap(),
+				&signature,
 				message.as_fixed_bytes(),
 			) {
 				Ok(pubkey) => {

pallets/zero-gas-transactions/src/tests.rs

@@ -56,3 +56,24 @@ fn fail_to_execute_transaction_with_high_nonce() {
 		));
 	})
 }
+
+#[test]
+fn fail_to_execute_transaction_with_short_validator_signature() {
+	new_test_ext().execute_with(|| {
+		let private_key = H256::random();
+		let trx1 = legacy_erc20_creation_transaction(0.into(), &private_key);
+
+		// A validator signature shorter than 65 bytes must be rejected, not panic.
+		let error = crate::Pallet::<Runtime>::send_zero_gas_transaction(
+			RawOrigin::None.into(),
+			Transaction::Legacy(trx1.clone()),
+			vec![0u8; 10],
+		)
+		.unwrap_err();
+
+		assert!(matches!(
+			error.error,
+			sp_runtime::DispatchError::Other("Invalid zero gas transaction signature")
+		));
+	})
+}

precompiles/fee-rewards-vault-controller/src/lib.rs

@@ -392,7 +392,8 @@ where
 		}
 
 		handle.record_cost(RuntimeHelper::<Runtime>::db_write_gas_cost())?;
-		pallet_dnt_fee_controller::Pallet::<Runtime>::set_validator_percentage(percentage).unwrap();
+		pallet_dnt_fee_controller::Pallet::<Runtime>::set_validator_percentage(percentage)
+			.map_err(|_| revert("percentage is too high"))?;
 
 		handle.record_log_costs_manual(1, 32)?;
 		log1(