diff --git a/CHANGELOG.md b/CHANGELOG.md index 549522c8f3..4a3a0b0f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ #### Upcoming Changes +* refactor: Limit ret opcode decodeing to Cairo0's standards. [#1925](https://github.com/lambdaclass/cairo-vm/pull/1925) + #### [2.0.0-rc4] - 2025-01-23 * feat: implement `kzg` data availability hints [#1887](https://github.com/lambdaclass/cairo-vm/pull/1887) diff --git a/vm/src/vm/decoding/decoder.rs b/vm/src/vm/decoding/decoder.rs index 35a98e7812..27b16875fe 100644 --- a/vm/src/vm/decoding/decoder.rs +++ b/vm/src/vm/decoding/decoder.rs @@ -82,11 +82,11 @@ pub fn decode_instruction(encoded_instr: u64) -> Result return Err(VirtualMachineError::InvalidPcUpdate(pc_update_num)), }; - let res = match res_logic_num { - 0 if matches!(pc_update, PcUpdate::Jnz) => Res::Unconstrained, - 0 => Res::Op1, - 1 => Res::Add, - 2 => Res::Mul, + let res = match (res_logic_num, pc_update == PcUpdate::Jnz) { + (0, true) => Res::Unconstrained, + (0, false) => Res::Op1, + (1, false) => Res::Add, + (2, false) => Res::Mul, _ => return Err(VirtualMachineError::InvalidRes(res_logic_num)), }; @@ -98,17 +98,38 @@ pub fn decode_instruction(encoded_instr: u64) -> Result return Err(VirtualMachineError::InvalidOpcode(opcode_num)), }; - let ap_update = match ap_update_num { - 0 if matches!(opcode, Opcode::Call) => ApUpdate::Add2, - 0 => ApUpdate::Regular, - 1 => ApUpdate::Add, - 2 => ApUpdate::Add1, + let ap_update = match (ap_update_num, opcode == Opcode::Call) { + (0, true) => ApUpdate::Add2, + (0, false) => ApUpdate::Regular, + (1, false) => ApUpdate::Add, + (2, false) => ApUpdate::Add1, _ => return Err(VirtualMachineError::InvalidApUpdate(ap_update_num)), }; let fp_update = match opcode { - Opcode::Call => FpUpdate::APPlus2, - Opcode::Ret => FpUpdate::Dst, + Opcode::Call => { + if off0 != 0 + || off1 != 1 + || ap_update != ApUpdate::Add2 + || dst_register != Register::AP + || op0_register != Register::AP + { + return Err(VirtualMachineError::InvalidOpcode(opcode_num)); + }; + FpUpdate::APPlus2 + } + Opcode::Ret => { + if off0 != -2 + || off2 != -1 + || dst_register != Register::FP + || op1_addr != Op1Addr::FP + || res != Res::Op1 + || pc_update != PcUpdate::Jump + { + return Err(VirtualMachineError::InvalidOpcode(opcode_num)); + }; + FpUpdate::Dst + } _ => FpUpdate::Regular, }; diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index 65209c35de..07ee837cde 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -154,7 +154,7 @@ pub struct CairoRunner { run_ended: bool, segments_finalized: bool, execution_public_memory: Option>, - runner_mode: RunnerMode, + pub(crate) runner_mode: RunnerMode, pub relocated_memory: Vec>, pub exec_scopes: ExecutionScopes, pub relocated_trace: Option>, diff --git a/vm/src/vm/security.rs b/vm/src/vm/security.rs index 2967a641bf..ff8c57f907 100644 --- a/vm/src/vm/security.rs +++ b/vm/src/vm/security.rs @@ -4,9 +4,10 @@ use num_traits::ToPrimitive; use super::{ errors::{runner_errors::RunnerError, vm_errors::VirtualMachineError}, - runners::cairo_runner::CairoRunner, + runners::cairo_runner::{CairoRunner, RunnerMode}, }; use crate::types::relocatable::MaybeRelocatable; +use crate::Felt252; /// Verify that the completed run in a runner is safe to be relocated and be /// used by other Cairo programs. @@ -79,6 +80,46 @@ pub fn verify_secure_runner( builtin.run_security_checks(&runner.vm)?; } + // Validate ret FP. + let initial_fp = runner.get_initial_fp().ok_or_else(|| { + VirtualMachineError::Other(anyhow::anyhow!( + "Failed to retrieve the initial_fp: it is None. \ + The initial_fp field should be initialized after running the entry point." + )) + })?; + let ret_fp_addr = (initial_fp - 2).map_err(VirtualMachineError::Math)?; + let ret_fp = runner.vm.get_maybe(&ret_fp_addr).ok_or_else(|| { + VirtualMachineError::Other(anyhow::anyhow!( + "Ret FP address is not in memory: {ret_fp_addr}" + )) + })?; + let final_fp = runner.vm.get_fp(); + if final_fp.segment_index != 1 { + return Err(VirtualMachineError::Other(anyhow::anyhow!( + "Final FP segment index is not 1: {final_fp}" + ))); + } + match ret_fp { + MaybeRelocatable::RelocatableValue(value) => { + if runner.runner_mode == RunnerMode::ProofModeCanonical && value != final_fp { + return Err(VirtualMachineError::Other(anyhow::anyhow!( + "Return FP is not equal to final FP: ret_f={ret_fp}, final_fp={final_fp}" + ))); + } + if runner.runner_mode == RunnerMode::ExecutionMode && value.offset != final_fp.offset { + return Err(VirtualMachineError::Other(anyhow::anyhow!( + "Return FP offset is not equal to final FP offset: ret_f={ret_fp}, final_fp={final_fp}" + ))); + } + } + MaybeRelocatable::Int(value) => { + if Felt252::from(final_fp.offset) != value { + return Err(VirtualMachineError::Other(anyhow::anyhow!( + "Return FP offset is not equal to final FP offset: ret_fp={ret_fp}, final_fp={final_fp}" + ))); + } + } + } Ok(()) }