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

[contracts] Add debug buffer limit + enforcement #12845

Merged
merged 7 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
67 changes: 59 additions & 8 deletions frame/contracts/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::{
use frame_support::{
crypto::ecdsa::ECDSAExt,
dispatch::{DispatchError, DispatchResult, DispatchResultWithPostInfo, Dispatchable},
ensure,
storage::{with_transaction, TransactionOutcome},
traits::{Contains, Currency, ExistenceRequirement, OriginTrait, Randomness, Time},
weights::Weight,
Expand Down Expand Up @@ -279,7 +280,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<bool, ()>;

/// Call some dispatchable and return the result.
fn call_runtime(&self, call: <Self::T as Config>::RuntimeCall) -> DispatchResultWithPostInfo;
Expand Down Expand Up @@ -1328,14 +1329,19 @@ 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<bool, ()> {
athei marked this conversation as resolved.
Show resolved Hide resolved
if let Some(buffer) = &mut self.debug_message {
if !msg.is_empty() {
ensure!(
buffer.len().checked_add(msg.len()).ok_or(())? <=
self.schedule.limits.debug_buffer_len as usize,
()
);
athei marked this conversation as resolved.
Show resolved Hide resolved
buffer.extend(msg.as_bytes());
}
true
Ok(true)
} else {
false
Ok(false)
}
}

Expand Down Expand Up @@ -2503,8 +2509,12 @@ 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()
});

Expand Down Expand Up @@ -2537,8 +2547,12 @@ mod tests {
#[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()
});

Expand Down Expand Up @@ -2568,6 +2582,43 @@ mod tests {
assert_eq!(&String::from_utf8(debug_buffer).unwrap(), "This is a testMore text");
}

#[test]
fn debug_buffer_is_limited() {
let schedule: Schedule<Test> = <Test as Config>::Schedule::get();
let code_hash = MockLoader::insert(Call, move |ctx, _| {
ctx.ext.append_debug_buffer("overflowing bytes").map_err(|_| {
DispatchError::Other("Maximum allowed debug buffer size exhausted!")
})?;
exec_success()
});

// Pre-fill the buffer up to its limit
let mut debug_buffer = vec![0u8; schedule.limits.debug_buffer_len as usize];

ExtBuilder::default().build().execute_with(|| {
let min_balance = <Test as Config>::Currency::minimum_balance();
let mut gas_meter = GasMeter::<Test>::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),
DispatchError::Other("Maximum allowed debug buffer size exhausted!")
);
});
}

#[test]
fn call_reentry_direct_recursion() {
// call the contract passed as input with disabled reentry
Expand Down
4 changes: 4 additions & 0 deletions frame/contracts/src/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ pub struct Limits {

/// The maximum size of a storage value and event payload in bytes.
pub payload_len: u32,

/// The maximum size of the debug buffer in bytes.
pub debug_buffer_len: u32,
}

impl Limits {
Expand Down Expand Up @@ -541,6 +544,7 @@ impl Default for Limits {
subject_len: 32,
call_depth: 32,
payload_len: 16 * 1024,
debug_buffer_len: 2 * 1024 * 1024,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,9 +594,9 @@ mod tests {
fn gas_meter(&mut self) -> &mut GasMeter<Self::T> {
&mut self.gas_meter
}
fn append_debug_buffer(&mut self, msg: &str) -> bool {
fn append_debug_buffer(&mut self, msg: &str) -> Result<bool, ()> {
self.debug_buffer.extend(msg.as_bytes());
true
Ok(true)
}
fn call_runtime(
&self,
Expand Down
4 changes: 2 additions & 2 deletions frame/contracts/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2395,11 +2395,11 @@ pub mod env {
str_len: u32,
) -> Result<ReturnCode, TrapReason> {
ctx.charge_gas(RuntimeCosts::DebugMessage)?;
if ctx.ext.append_debug_buffer("") {
if ctx.ext.append_debug_buffer("").map_err(|_| Error::<E::T>::OutOfBounds)? {
let data = ctx.read_sandbox_memory(memory, str_ptr, str_len)?;
let msg =
core::str::from_utf8(&data).map_err(|_| <Error<E::T>>::DebugMessageInvalidUTF8)?;
ctx.ext.append_debug_buffer(msg);
ctx.ext.append_debug_buffer(msg).map_err(|_| Error::<E::T>::OutOfBounds)?;
return Ok(ReturnCode::Success)
}
Ok(ReturnCode::LoggingDisabled)
Expand Down