From fcf6a2a4f5a139d5425e3ed1115a41ea1a4bc285 Mon Sep 17 00:00:00 2001 From: Hannes Karppila Date: Thu, 9 Nov 2023 15:34:29 +0200 Subject: [PATCH] Limit ReceiptsCtx to u16::MAX receipts (#633) * Limit ReceiptsCtx to u16::MAX receipts * Changelog * Fix for features=alloc --- CHANGELOG.md | 1 + fuel-asm/src/panic_reason.rs | 2 + fuel-vm/src/error.rs | 4 ++ fuel-vm/src/interpreter/blockchain.rs | 6 +- fuel-vm/src/interpreter/contract.rs | 4 +- fuel-vm/src/interpreter/diff/tests.rs | 2 +- .../src/interpreter/executors/instruction.rs | 2 +- fuel-vm/src/interpreter/executors/main.rs | 2 +- fuel-vm/src/interpreter/flow.rs | 13 ++-- fuel-vm/src/interpreter/flow/ret_tests.rs | 2 +- fuel-vm/src/interpreter/internal.rs | 8 ++- fuel-vm/src/interpreter/log.rs | 4 +- fuel-vm/src/interpreter/receipts.rs | 43 ++++++++++-- fuel-vm/src/tests/mod.rs | 1 + fuel-vm/src/tests/receipts.rs | 70 +++++++++++++++++++ 15 files changed, 139 insertions(+), 25 deletions(-) create mode 100644 fuel-vm/src/tests/receipts.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ff3e88627..fcde7767af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - If the `max_fee > policies.max_fee`, then transaction will be rejected. - If the `witnessses_size > policies.witness_limit`, then transaction will be rejected. - GTF opcode changed its hardcoded constants for fields. It should be updated according to the values from the specification on the Sway side. +- [#633](https://github.com/FuelLabs/fuel-vm/pull/633): Limit receipt count to `u16::MAX`. ### Fixed diff --git a/fuel-asm/src/panic_reason.rs b/fuel-asm/src/panic_reason.rs index 13ac220725..375fe09136 100644 --- a/fuel-asm/src/panic_reason.rs +++ b/fuel-asm/src/panic_reason.rs @@ -119,6 +119,8 @@ enum_from! { PolicyIsNotSet = 0x28, /// The policy is not found across policies. PolicyNotFound = 0x29, + /// Receipt context is full + TooManyReceipts = 0x2a, } } diff --git a/fuel-vm/src/error.rs b/fuel-vm/src/error.rs index 904a1f55ed..a02c2c0b6a 100644 --- a/fuel-vm/src/error.rs +++ b/fuel-vm/src/error.rs @@ -353,6 +353,10 @@ pub enum BugVariant { /// Refund cannot be computed in the current vm state. #[strum(message = "Refund cannot be computed in the current vm state.")] UncomputableRefund, + + /// Receipts context is full, but there's an attempt to add more receipts. + #[strum(message = "Receipts context is full, cannot add new receipts.")] + ReceiptsCtxFull, } impl fmt::Display for BugVariant { diff --git a/fuel-vm/src/interpreter/blockchain.rs b/fuel-vm/src/interpreter/blockchain.rs index c0109897d1..c6b19a804d 100644 --- a/fuel-vm/src/interpreter/blockchain.rs +++ b/fuel-vm/src/interpreter/blockchain.rs @@ -622,7 +622,7 @@ where let receipt = Receipt::burn(*sub_id, *contract_id, a, *self.pc, *self.is); - append_receipt(self.append, receipt); + append_receipt(self.append, receipt)?; Ok(inc_pc(self.pc)?) } @@ -660,7 +660,7 @@ where let receipt = Receipt::mint(*sub_id, *contract_id, a, *self.pc, *self.is); - append_receipt(self.append, receipt); + append_receipt(self.append, receipt)?; Ok(inc_pc(self.pc)?) } @@ -1021,7 +1021,7 @@ where memory: self.memory, }, receipt, - ); + )?; Ok(inc_pc(self.pc)?) } diff --git a/fuel-vm/src/interpreter/contract.rs b/fuel-vm/src/interpreter/contract.rs index 0186cefffc..9a2e9c5a10 100644 --- a/fuel-vm/src/interpreter/contract.rs +++ b/fuel-vm/src/interpreter/contract.rs @@ -268,7 +268,7 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> { memory: self.memory, }, receipt, - ); + )?; Ok(inc_pc(self.pc)?) } @@ -340,7 +340,7 @@ impl<'vm, S, Tx> TransferCtx<'vm, S, Tx> { memory: self.memory, }, receipt, - ); + )?; Ok(inc_pc(self.pc)?) } diff --git a/fuel-vm/src/interpreter/diff/tests.rs b/fuel-vm/src/interpreter/diff/tests.rs index 613c067431..9e86b25b3a 100644 --- a/fuel-vm/src/interpreter/diff/tests.rs +++ b/fuel-vm/src/interpreter/diff/tests.rs @@ -126,7 +126,7 @@ fn reset_vm_state_receipts() { Default::default(), Default::default(), ); - b.receipts.push(receipt); + b.receipts.push(receipt).expect("not full"); assert_ne!(a.receipts, b.receipts); let diff: Diff = a.diff(&b).into(); b.reset_vm_state(&diff); diff --git a/fuel-vm/src/interpreter/executors/instruction.rs b/fuel-vm/src/interpreter/executors/instruction.rs index 2f621affed..ab298f96cd 100644 --- a/fuel-vm/src/interpreter/executors/instruction.rs +++ b/fuel-vm/src/interpreter/executors/instruction.rs @@ -600,7 +600,7 @@ where self.gas_charge(self.gas_costs().rvrt)?; let a = rvrt.unpack(); let ra = r!(a); - self.revert(ra); + self.revert(ra)?; return Ok(ExecuteState::Revert(ra)) } diff --git a/fuel-vm/src/interpreter/executors/main.rs b/fuel-vm/src/interpreter/executors/main.rs index cc3207c7fd..3ac434c76a 100644 --- a/fuel-vm/src/interpreter/executors/main.rs +++ b/fuel-vm/src/interpreter/executors/main.rs @@ -560,7 +560,7 @@ where let receipt = Receipt::script_result(status, gas_used); - self.append_receipt(receipt); + self.append_receipt(receipt)?; if program.is_debug() { self.debugger_set_last_state(program); diff --git a/fuel-vm/src/interpreter/flow.rs b/fuel-vm/src/interpreter/flow.rs index c2991deba6..463af3b7b0 100644 --- a/fuel-vm/src/interpreter/flow.rs +++ b/fuel-vm/src/interpreter/flow.rs @@ -137,7 +137,7 @@ where input.ret_data(a, b) } - pub(crate) fn revert(&mut self, a: Word) { + pub(crate) fn revert(&mut self, a: Word) -> SimpleResult<()> { let current_contract = current_contract(&self.context, self.registers.fp(), self.memory.as_ref()) .map_or_else(|_| Some(ContractId::zeroed()), Option::<&_>::copied); @@ -172,7 +172,8 @@ where }; self.panic_context = PanicContext::None; - self.append_receipt(receipt); + self.append_receipt(receipt) + .expect("Appending a panic receipt cannot fail"); } } @@ -227,7 +228,7 @@ impl RetCtx<'_> { set_frame_pointer(context, registers.fp_mut(), fp); } - append_receipt(self.append, receipt); + append_receipt(self.append, receipt)?; Ok(inc_pc(self.registers.pc_mut())?) } @@ -261,7 +262,7 @@ pub(crate) fn revert( pc: Reg, is: Reg, a: Word, -) { +) -> SimpleResult<()> { let receipt = Receipt::revert( current_contract.unwrap_or_else(ContractId::zeroed), a, @@ -269,7 +270,7 @@ pub(crate) fn revert( *is, ); - append_receipt(append, receipt); + append_receipt(append, receipt) } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -628,7 +629,7 @@ where memory: self.memory.memory, }, receipt, - ); + )?; self.frames.push(frame); diff --git a/fuel-vm/src/interpreter/flow/ret_tests.rs b/fuel-vm/src/interpreter/flow/ret_tests.rs index af95da8842..3801093ac4 100644 --- a/fuel-vm/src/interpreter/flow/ret_tests.rs +++ b/fuel-vm/src/interpreter/flow/ret_tests.rs @@ -179,7 +179,7 @@ fn test_revert() { }; let pc = 10; let is = 20; - revert(append, None, Reg::new(&pc), Reg::new(&is), 99); + revert(append, None, Reg::new(&pc), Reg::new(&is), 99).expect("should be ok"); assert_eq!( *receipts.as_ref().last().unwrap(), Receipt::revert(ContractId::default(), 99, pc, is) diff --git a/fuel-vm/src/interpreter/internal.rs b/fuel-vm/src/interpreter/internal.rs index f286c80faa..ab7ad92322 100644 --- a/fuel-vm/src/interpreter/internal.rs +++ b/fuel-vm/src/interpreter/internal.rs @@ -55,7 +55,7 @@ where update_memory_output(&mut self.tx, &mut self.memory, tx_offset, idx) } - pub(crate) fn append_receipt(&mut self, receipt: Receipt) { + pub(crate) fn append_receipt(&mut self, receipt: Receipt) -> SimpleResult<()> { let tx_offset = self.tx_offset(); append_receipt( AppendReceipt { @@ -128,14 +128,14 @@ pub(crate) struct AppendReceipt<'vm> { pub memory: &'vm mut [u8; MEM_SIZE], } -pub(crate) fn append_receipt(input: AppendReceipt, receipt: Receipt) { +pub(crate) fn append_receipt(input: AppendReceipt, receipt: Receipt) -> SimpleResult<()> { let AppendReceipt { receipts, script, tx_offset, memory, } = input; - receipts.push(receipt); + receipts.push(receipt)?; if let Some(script) = script { let offset = tx_offset + script.receipts_root_offset(); @@ -151,6 +151,8 @@ pub(crate) fn append_receipt(input: AppendReceipt, receipt: Receipt) { // guaranteed to fit memory[offset..offset + Bytes32::LEN].copy_from_slice(&root[..]); } + + Ok(()) } impl Interpreter { diff --git a/fuel-vm/src/interpreter/log.rs b/fuel-vm/src/interpreter/log.rs index 8f5d01a64d..01907af6b1 100644 --- a/fuel-vm/src/interpreter/log.rs +++ b/fuel-vm/src/interpreter/log.rs @@ -102,7 +102,7 @@ impl LogInput<'_> { memory: self.memory, }, receipt, - ); + )?; Ok(inc_pc(self.pc)?) } @@ -128,7 +128,7 @@ impl LogInput<'_> { memory: self.memory, }, receipt, - ); + )?; Ok(inc_pc(self.pc)?) } diff --git a/fuel-vm/src/interpreter/receipts.rs b/fuel-vm/src/interpreter/receipts.rs index 6e882413d6..4940f8b0a8 100644 --- a/fuel-vm/src/interpreter/receipts.rs +++ b/fuel-vm/src/interpreter/receipts.rs @@ -3,6 +3,7 @@ use core::{ mem, ops::Index, }; +use fuel_asm::PanicReason; use fuel_merkle::binary::root_calculator::MerkleRootCalculator as MerkleTree; use fuel_tx::Receipt; @@ -11,6 +12,14 @@ use fuel_types::{ Bytes32, }; +use crate::{ + error::SimpleResult, + prelude::{ + Bug, + BugVariant, + }, +}; + /// Receipts and the associated Merkle tree #[derive(Debug, Default, Clone)] pub struct ReceiptsCtx { @@ -19,10 +28,33 @@ pub struct ReceiptsCtx { } impl ReceiptsCtx { - /// Add a new receipt, updating the Merkle tree as well - pub fn push(&mut self, receipt: Receipt) { + /// The maximum number of receipts that can be stored in a single context. + /// https://github.com/FuelLabs/fuel-specs/blob/master/src/fuel-vm/instruction-set.md#Receipts + pub const MAX_RECEIPTS: usize = u16::MAX as usize; + + /// Add a new receipt, updating the Merkle tree as well. + /// Returns a panic if the context is full. + pub fn push(&mut self, receipt: Receipt) -> SimpleResult<()> { + if self.receipts.len() == Self::MAX_RECEIPTS { + return Err(Bug::new(BugVariant::ReceiptsCtxFull).into()) + } + + // Last two slots can be only used for ending the script, + // with a script result optinally preceded by a panic + if (self.receipts.len() == Self::MAX_RECEIPTS - 1 + && !matches!(receipt, Receipt::ScriptResult { .. })) + || (self.receipts.len() == Self::MAX_RECEIPTS - 2 + && !matches!( + receipt, + Receipt::ScriptResult { .. } | Receipt::Panic { .. } + )) + { + return Err(PanicReason::TooManyReceipts.into()) + } + self.receipts_tree.push(receipt.to_bytes().as_slice()); - self.receipts.push(receipt) + self.receipts.push(receipt); + Ok(()) } /// Reset the context to an empty state @@ -83,11 +115,12 @@ impl PartialEq for ReceiptsCtx { impl Eq for ReceiptsCtx {} +#[cfg(any(test, feature = "test-helpers"))] impl From> for ReceiptsCtx { fn from(receipts: Vec) -> Self { let mut ctx = Self::default(); for receipt in receipts { - ctx.push(receipt) + ctx.push(receipt).expect("Too many receipts"); } ctx } @@ -158,7 +191,7 @@ mod tests { let mut ctx = ReceiptsCtx::default(); let receipts = iter::repeat(create_receipt()).take(5); for receipt in receipts.clone() { - ctx.push(receipt) + ctx.push(receipt).expect("context not full"); } let root = ctx.root(); diff --git a/fuel-vm/src/tests/mod.rs b/fuel-vm/src/tests/mod.rs index 1ed99daa91..4a24e15e63 100644 --- a/fuel-vm/src/tests/mod.rs +++ b/fuel-vm/src/tests/mod.rs @@ -24,6 +24,7 @@ mod metadata; mod outputs; mod predicate; mod profile_gas; +mod receipts; mod serde_profile; mod spec; mod validation; diff --git a/fuel-vm/src/tests/receipts.rs b/fuel-vm/src/tests/receipts.rs new file mode 100644 index 0000000000..23a3218374 --- /dev/null +++ b/fuel-vm/src/tests/receipts.rs @@ -0,0 +1,70 @@ +use fuel_asm::{ + op, + PanicReason, + RegId, +}; +use fuel_tx::{ + Receipt, + ScriptExecutionResult, +}; + +use alloc::vec; + +use crate::interpreter::ReceiptsCtx; + +use super::test_helpers::run_script; + +#[test] +fn too_many_receipts_panics() { + let receipts = run_script(vec![ + op::log(RegId::ZERO, RegId::ZERO, RegId::ZERO, RegId::ZERO), + op::jmpb(RegId::ZERO, 0), + ]); + + assert_eq!(receipts.len(), ReceiptsCtx::MAX_RECEIPTS); + + // The panic receipt should have still been pushed correctly + let Receipt::Panic { reason, .. } = receipts[ReceiptsCtx::MAX_RECEIPTS - 2] else { + panic!("Expect panic receipt"); + }; + assert_eq!(*reason.reason(), PanicReason::TooManyReceipts); +} + +#[test] +fn can_panic_just_before_max_receipts() { + let receipts = run_script(vec![ + op::movi(0x10, ReceiptsCtx::MAX_RECEIPTS as u32 - 2), + op::log(RegId::ZERO, RegId::ZERO, RegId::ZERO, RegId::ZERO), + op::subi(0x10, 0x10, 1), + op::jnzb(0x10, RegId::ZERO, 1), + op::div(0x10, RegId::ZERO, RegId::ZERO), // Divide by zero + ]); + + assert_eq!(receipts.len(), ReceiptsCtx::MAX_RECEIPTS); + + // The panic receipt should have still been pushed correctly + let Receipt::Panic { reason, .. } = receipts[ReceiptsCtx::MAX_RECEIPTS - 2] else { + panic!("Expect panic receipt"); + }; + assert_eq!(*reason.reason(), PanicReason::ArithmeticError); +} + +#[test] +fn can_return_successfully_just_below_max_receipts() { + let receipts = run_script(vec![ + op::movi(0x10, ReceiptsCtx::MAX_RECEIPTS as u32 - 3), + op::log(RegId::ZERO, RegId::ZERO, RegId::ZERO, RegId::ZERO), + op::subi(0x10, 0x10, 1), + op::jnzb(0x10, RegId::ZERO, 1), + op::ret(RegId::ONE), + ]); + + assert_eq!(receipts.len(), ReceiptsCtx::MAX_RECEIPTS - 1); + + // The panic receipt should have still been pushed correctly + let Receipt::ScriptResult { result, .. } = receipts[ReceiptsCtx::MAX_RECEIPTS - 2] + else { + panic!("Expect result receipt"); + }; + assert_eq!(result, ScriptExecutionResult::Success); +}