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
Changes from 1 commit
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
Next Next commit
Add ContractAccessWeight
athei committed May 6, 2022

Verified

This commit was signed with the committer’s verified signature.
athei Alexander Theißen
commit 9d679322227e8a7442f0353c08433f365ec0b3dd
8 changes: 6 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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 {
50 changes: 47 additions & 3 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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.
athei marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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 charged fees for computation incurred but not for PoV
athei marked this conversation as resolved.
Show resolved Hide resolved
/// 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
5 changes: 3 additions & 2 deletions frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
@@ -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]);
10 changes: 7 additions & 3 deletions frame/contracts/src/wasm/code_cache.rs
Original file line number Diff line number Diff line change
@@ -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)
},
}
}
}