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

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

Merged
merged 2 commits into from
May 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() / (
Expand Down Expand Up @@ -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 {
Expand Down
50 changes: 47 additions & 3 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>;
Expand Down Expand Up @@ -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::*;
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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]);
Expand Down
10 changes: 7 additions & 3 deletions frame/contracts/src/wasm/code_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
},
}
}
}