diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 7cd42be73a19b..00d2a54d1e774 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1216,6 +1216,7 @@ impl pallet_contracts::Config for Runtime { type MaxCodeLen = ConstU32<{ 128 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = ConstBool; + type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; } impl pallet_sudo::Config for Runtime { diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index c0cf6a9f4c4c4..945095dc20329 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -18,8 +18,8 @@ use crate::{ gas::GasMeter, storage::{self, Storage, WriteOutcome}, - BalanceOf, CodeHash, Config, ContractInfo, ContractInfoOf, Determinism, Error, Event, Nonce, - Pallet as Contracts, Schedule, + BalanceOf, CodeHash, Config, ContractInfo, ContractInfoOf, DebugBufferVec, Determinism, Error, + Event, Nonce, Pallet as Contracts, Schedule, }; use frame_support::{ crypto::ecdsa::ECDSAExt, @@ -279,7 +279,7 @@ pub trait Ext: sealing::Sealed { /// when the code is executing on-chain. /// /// Returns `true` if debug message recording is enabled. Otherwise `false` is returned. - fn append_debug_buffer(&mut self, msg: &str) -> bool; + fn append_debug_buffer(&mut self, msg: &str) -> Result; /// Call some dispatchable and return the result. fn call_runtime(&self, call: ::RuntimeCall) -> DispatchResultWithPostInfo; @@ -409,7 +409,7 @@ pub struct Stack<'a, T: Config, E> { /// /// All the bytes added to this field should be valid UTF-8. The buffer has no defined /// structure and is intended to be shown to users as-is for debugging purposes. - debug_message: Option<&'a mut Vec>, + debug_message: Option<&'a mut DebugBufferVec>, /// The determinism requirement of this call stack. determinism: Determinism, /// No executable is held by the struct but influences its behaviour. @@ -617,7 +617,7 @@ where schedule: &'a Schedule, value: BalanceOf, input_data: Vec, - debug_message: Option<&'a mut Vec>, + debug_message: Option<&'a mut DebugBufferVec>, determinism: Determinism, ) -> Result { let (mut stack, executable) = Self::new( @@ -652,7 +652,7 @@ where value: BalanceOf, input_data: Vec, salt: &[u8], - debug_message: Option<&'a mut Vec>, + debug_message: Option<&'a mut DebugBufferVec>, ) -> Result<(T::AccountId, ExecReturnValue), ExecError> { let (mut stack, executable) = Self::new( FrameArgs::Instantiate { @@ -681,7 +681,7 @@ where storage_meter: &'a mut storage::meter::Meter, schedule: &'a Schedule, value: BalanceOf, - debug_message: Option<&'a mut Vec>, + debug_message: Option<&'a mut DebugBufferVec>, determinism: Determinism, ) -> Result<(Self, E), ExecError> { let (first_frame, executable, nonce) = Self::new_frame( @@ -1328,14 +1328,16 @@ where &mut self.top_frame_mut().nested_gas } - fn append_debug_buffer(&mut self, msg: &str) -> bool { + fn append_debug_buffer(&mut self, msg: &str) -> Result { if let Some(buffer) = &mut self.debug_message { if !msg.is_empty() { - buffer.extend(msg.as_bytes()); + buffer + .try_extend(&mut msg.bytes()) + .map_err(|_| Error::::DebugBufferExhausted)?; } - true + Ok(true) } else { - false + Ok(false) } } @@ -2503,12 +2505,16 @@ mod tests { #[test] fn printing_works() { let code_hash = MockLoader::insert(Call, |ctx, _| { - ctx.ext.append_debug_buffer("This is a test"); - ctx.ext.append_debug_buffer("More text"); + ctx.ext + .append_debug_buffer("This is a test") + .expect("Maximum allowed debug buffer size exhausted!"); + ctx.ext + .append_debug_buffer("More text") + .expect("Maximum allowed debug buffer size exhausted!"); exec_success() }); - let mut debug_buffer = Vec::new(); + let mut debug_buffer = DebugBufferVec::::try_from(Vec::new()).unwrap(); ExtBuilder::default().build().execute_with(|| { let min_balance = ::Currency::minimum_balance(); @@ -2531,18 +2537,22 @@ mod tests { .unwrap(); }); - assert_eq!(&String::from_utf8(debug_buffer).unwrap(), "This is a testMore text"); + assert_eq!(&String::from_utf8(debug_buffer.to_vec()).unwrap(), "This is a testMore text"); } #[test] fn printing_works_on_fail() { let code_hash = MockLoader::insert(Call, |ctx, _| { - ctx.ext.append_debug_buffer("This is a test"); - ctx.ext.append_debug_buffer("More text"); + ctx.ext + .append_debug_buffer("This is a test") + .expect("Maximum allowed debug buffer size exhausted!"); + ctx.ext + .append_debug_buffer("More text") + .expect("Maximum allowed debug buffer size exhausted!"); exec_trapped() }); - let mut debug_buffer = Vec::new(); + let mut debug_buffer = DebugBufferVec::::try_from(Vec::new()).unwrap(); ExtBuilder::default().build().execute_with(|| { let min_balance = ::Currency::minimum_balance(); @@ -2565,7 +2575,43 @@ mod tests { assert!(result.is_err()); }); - assert_eq!(&String::from_utf8(debug_buffer).unwrap(), "This is a testMore text"); + assert_eq!(&String::from_utf8(debug_buffer.to_vec()).unwrap(), "This is a testMore text"); + } + + #[test] + fn debug_buffer_is_limited() { + let code_hash = MockLoader::insert(Call, move |ctx, _| { + ctx.ext.append_debug_buffer("overflowing bytes")?; + exec_success() + }); + + // Pre-fill the buffer up to its limit + let mut debug_buffer = + DebugBufferVec::::try_from(vec![0u8; DebugBufferVec::::bound()]).unwrap(); + + ExtBuilder::default().build().execute_with(|| { + let schedule: Schedule = ::Schedule::get(); + let min_balance = ::Currency::minimum_balance(); + let mut gas_meter = GasMeter::::new(GAS_LIMIT); + set_balance(&ALICE, min_balance * 10); + place_contract(&BOB, code_hash); + let mut storage_meter = storage::meter::Meter::new(&ALICE, Some(0), 0).unwrap(); + assert_err!( + MockStack::run_call( + ALICE, + BOB, + &mut gas_meter, + &mut storage_meter, + &schedule, + 0, + vec![], + Some(&mut debug_buffer), + Determinism::Deterministic, + ) + .map_err(|e| e.error), + Error::::DebugBufferExhausted + ); + }); } #[test] diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index 06d817785cc39..b76acf9d1db08 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -142,6 +142,7 @@ type BalanceOf = type CodeVec = BoundedVec::MaxCodeLen>; type RelaxedCodeVec = WeakBoundedVec::MaxCodeLen>; type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; +type DebugBufferVec = BoundedVec::MaxDebugBufferLen>; /// Used as a sentinel value when reading and writing contract memory. /// @@ -344,6 +345,10 @@ pub mod pallet { /// Do **not** set to `true` on productions chains. #[pallet::constant] type UnsafeUnstableInterface: Get; + + /// The maximum length of the debug buffer in bytes. + #[pallet::constant] + type MaxDebugBufferLen: Get; } #[pallet::hooks] @@ -863,6 +868,9 @@ pub mod pallet { CodeRejected, /// An indetermistic code was used in a context where this is not permitted. Indeterministic, + /// The debug buffer size used during contract execution exceeded the limit determined by + /// the `MaxDebugBufferLen` pallet config parameter. + DebugBufferExhausted, } /// A mapping from an original code hash to the original code, untouched by instrumentation. @@ -961,7 +969,7 @@ where debug: bool, determinism: Determinism, ) -> ContractExecResult> { - let mut debug_message = if debug { Some(Vec::new()) } else { None }; + let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let output = Self::internal_call( origin, dest, @@ -977,7 +985,7 @@ where gas_consumed: output.gas_meter.gas_consumed(), gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, - debug_message: debug_message.unwrap_or_default(), + debug_message: debug_message.unwrap_or_default().to_vec(), } } @@ -1003,7 +1011,7 @@ where salt: Vec, debug: bool, ) -> ContractInstantiateResult> { - let mut debug_message = if debug { Some(Vec::new()) } else { None }; + let mut debug_message = if debug { Some(DebugBufferVec::::default()) } else { None }; let output = Self::internal_instantiate( origin, value, @@ -1022,7 +1030,7 @@ where gas_consumed: output.gas_meter.gas_consumed(), gas_required: output.gas_meter.gas_required(), storage_deposit: output.storage_deposit, - debug_message: debug_message.unwrap_or_default(), + debug_message: debug_message.unwrap_or_default().to_vec(), } } @@ -1113,7 +1121,7 @@ where gas_limit: Weight, storage_deposit_limit: Option>, data: Vec, - debug_message: Option<&mut Vec>, + debug_message: Option<&mut DebugBufferVec>, determinism: Determinism, ) -> InternalCallOutput { let mut gas_meter = GasMeter::new(gas_limit); @@ -1156,7 +1164,7 @@ where code: Code>, data: Vec, salt: Vec, - mut debug_message: Option<&mut Vec>, + mut debug_message: Option<&mut DebugBufferVec>, ) -> InternalInstantiateOutput { let mut storage_deposit = Default::default(); let mut gas_meter = GasMeter::new(gas_limit); @@ -1172,7 +1180,7 @@ where TryInstantiate::Skip, ) .map_err(|(err, msg)| { - debug_message.as_mut().map(|buffer| buffer.extend(msg.as_bytes())); + debug_message.as_mut().map(|buffer| buffer.try_extend(&mut msg.bytes())); err })?; // The open deposit will be charged during execution when the diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index a467800dfe15b..6121d880ca8c5 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -415,6 +415,7 @@ impl Config for Test { type MaxCodeLen = ConstU32<{ 128 * 1024 }>; type MaxStorageKeyLen = ConstU32<128>; type UnsafeUnstableInterface = UnstableInterface; + type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>; } pub const ALICE: AccountId32 = AccountId32::new([1u8; 32]); diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index e9e6b42dc3f8a..d85dac95cc712 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -594,9 +594,9 @@ mod tests { fn gas_meter(&mut self) -> &mut GasMeter { &mut self.gas_meter } - fn append_debug_buffer(&mut self, msg: &str) -> bool { + fn append_debug_buffer(&mut self, msg: &str) -> Result { self.debug_buffer.extend(msg.as_bytes()); - true + Ok(true) } fn call_runtime( &self, diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index b933688eb61ec..50ad9996e6eb6 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -2395,11 +2395,11 @@ pub mod env { str_len: u32, ) -> Result { ctx.charge_gas(RuntimeCosts::DebugMessage)?; - if ctx.ext.append_debug_buffer("") { + if ctx.ext.append_debug_buffer("")? { let data = ctx.read_sandbox_memory(memory, str_ptr, str_len)?; let msg = core::str::from_utf8(&data).map_err(|_| >::DebugMessageInvalidUTF8)?; - ctx.ext.append_debug_buffer(msg); + ctx.ext.append_debug_buffer(msg)?; return Ok(ReturnCode::Success) } Ok(ReturnCode::LoggingDisabled)