contracts: Prevent PoV attack vector with minimal viable solution (#11372)

* Add ContractAccessWeight

* Apply suggestions from code review

Co-authored-by: Michael Müller <michi@parity.io>

Co-authored-by: Michael Müller <michi@parity.io>

substrate/bin/node/runtime/src/lib.rs

@@ -1059,8 +1059,11 @@ parameter_types! {
 	pub const DepositPerByte: Balance = deposit(0, 1);
 	pub const MaxValueSize: u32 = 16 * 1024;
 	// The lazy deletion runs inside on_initialize.
-	pub DeletionWeightLimit: Weight = AVERAGE_ON_INITIALIZE_RATIO *
-		RuntimeBlockWeights::get().max_block;
+	pub DeletionWeightLimit: Weight = RuntimeBlockWeights::get()
+		.per_class
+		.get(DispatchClass::Normal)
+		.max_total
+		.unwrap_or(RuntimeBlockWeights::get().max_block);
 	// The weight needed for decoding the queue should be less or equal than a fifth
 	// of the overall weight dedicated to the lazy deletion.
 	pub DeletionQueueDepth: u32 = ((DeletionWeightLimit::get() / (
@@ -1093,6 +1096,7 @@ impl pallet_contracts::Config for Runtime {
 	type DeletionWeightLimit = DeletionWeightLimit;
 	type Schedule = Schedule;
 	type AddressGenerator = pallet_contracts::DefaultAddressGenerator;
+	type ContractAccessWeight = pallet_contracts::DefaultContractAccessWeight<RuntimeBlockWeights>;
 }
 
 impl pallet_sudo::Config for Runtime {

substrate/frame/contracts/src/lib.rs

@@ -115,9 +115,9 @@ use frame_support::{
 	dispatch::Dispatchable,
 	ensure,
 	traits::{Contains, Currency, Get, Randomness, ReservableCurrency, StorageVersion, Time},
-	weights::{GetDispatchInfo, Pays, PostDispatchInfo, Weight},
+	weights::{DispatchClass, GetDispatchInfo, Pays, PostDispatchInfo, Weight},
 };
-use frame_system::Pallet as System;
+use frame_system::{limits::BlockWeights, Pallet as System};
 use pallet_contracts_primitives::{
 	Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult,
 	ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue,
@@ -126,7 +126,7 @@ use pallet_contracts_primitives::{
 use scale_info::TypeInfo;
 use sp_core::{crypto::UncheckedFrom, Bytes};
 use sp_runtime::traits::{Convert, Hash, Saturating, StaticLookup};
-use sp_std::{fmt::Debug, prelude::*};
+use sp_std::{fmt::Debug, marker::PhantomData, prelude::*};
 
 type CodeHash<T> = <T as frame_system::Config>::Hash;
 type TrieId = Vec<u8>;
@@ -193,6 +193,29 @@ where
 	}
 }
 
+/// A conservative implementation to be used for [`pallet::Config::ContractAccessWeight`].
+///
+/// This derives the weight from the [`BlockWeights`] passed as `B` and the `maxPovSize` passed
+/// as `P`. The default value for `P` is the `maxPovSize` used by Polkadot and Kusama.
+///
+/// It simply charges from the weight meter pro rata: If loading the contract code would consume
+/// 50% of the max storage proof then this charges 50% of the max block weight.
+pub struct DefaultContractAccessWeight<B: Get<BlockWeights>, const P: u32 = 5_242_880>(
+	PhantomData<B>,
+);
+
+impl<B: Get<BlockWeights>, const P: u32> Get<Weight> for DefaultContractAccessWeight<B, P> {
+	fn get() -> Weight {
+		let block_weights = B::get();
+		block_weights
+			.per_class
+			.get(DispatchClass::Normal)
+			.max_total
+			.unwrap_or(block_weights.max_block) /
+			Weight::from(P)
+	}
+}
+
 #[frame_support::pallet]
 pub mod pallet {
 	use super::*;
@@ -297,6 +320,27 @@ pub mod pallet {
 		#[pallet::constant]
 		type DepositPerByte: Get<BalanceOf<Self>>;
 
+		/// The weight per byte of code that is charged when loading a contract from storage.
+		///
+		/// Currently, FRAME only charges fees for computation incurred but not for PoV
+		/// consumption caused for storage access. This is usually not exploitable because
+		/// accessing storage carries some substantial weight costs, too. However in case
+		/// of contract code very much PoV consumption can be caused while consuming very little
+		/// computation. This could be used to keep the chain busy without paying the
+		/// proper fee for it. Until this is resolved we charge from the weight meter for
+		/// contract access.
+		///
+		/// For more information check out: <https://github.com/paritytech/substrate/issues/10301>
+		///
+		/// [`DefaultContractAccessWeight`] is a safe default to be used for polkadot or kusama
+		/// parachains.
+		///
+		/// # Note
+		///
+		/// This is only relevant for parachains. Set to zero in case of a standalone chain.
+		#[pallet::constant]
+		type ContractAccessWeight: Get<Weight>;
+
 		/// The amount of balance a caller has to pay for each storage item.
 		///
 		/// # Note

substrate/frame/contracts/src/tests.rs

@@ -24,8 +24,8 @@ use crate::{
 	storage::Storage,
 	wasm::{PrefabWasmModule, ReturnCode as RuntimeReturnCode},
 	weights::WeightInfo,
-	BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator, Error, Pallet,
-	Schedule,
+	BalanceOf, Code, CodeStorage, Config, ContractInfoOf, DefaultAddressGenerator,
+	DefaultContractAccessWeight, Error, Pallet, Schedule,
 };
 use assert_matches::assert_matches;
 use codec::Encode;
@@ -287,6 +287,7 @@ impl Config for Test {
 	type DepositPerByte = DepositPerByte;
 	type DepositPerItem = DepositPerItem;
 	type AddressGenerator = DefaultAddressGenerator;
+	type ContractAccessWeight = DefaultContractAccessWeight<BlockWeights>;
 }
 
 pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]);

substrate/frame/contracts/src/wasm/code_cache.rs

@@ -38,7 +38,7 @@ use crate::{
 use frame_support::{
 	dispatch::{DispatchError, DispatchResult},
 	ensure,
-	traits::ReservableCurrency,
+	traits::{Get, ReservableCurrency},
 };
 use sp_core::crypto::UncheckedFrom;
 use sp_runtime::traits::BadOrigin;
@@ -216,8 +216,12 @@ impl<T: Config> Token<T> for CodeToken {
 		// size of the contract.
 		match *self {
 			Reinstrument(len) => T::WeightInfo::reinstrument(len),
-			Load(len) => T::WeightInfo::call_with_code_per_byte(len)
-				.saturating_sub(T::WeightInfo::call_with_code_per_byte(0)),
+			Load(len) => {
+				let computation = T::WeightInfo::call_with_code_per_byte(len)
+					.saturating_sub(T::WeightInfo::call_with_code_per_byte(0));
+				let bandwith = T::ContractAccessWeight::get().saturating_mul(len.into());
+				computation.max(bandwith)
+			},
 		}
 	}
 }