From c2cf08b9039913f979cc4a8acb300b6eb227f232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20Calv=C3=ADn=20Garc=C3=ADa?= Date: Mon, 4 Dec 2023 22:18:02 +0100 Subject: [PATCH] Add bounds for out of range check in get_block_hash syscall (#1167) * add bounds for out of range check in get_block_hash syscall * fix possible panick in get_block_hash condition --- .../business_logic_syscall_handler.rs | 2 +- src/syscalls/native_syscall_handler.rs | 9 +- tests/cairo_native.rs | 106 ++++++++++++++++++ 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/src/syscalls/business_logic_syscall_handler.rs b/src/syscalls/business_logic_syscall_handler.rs index 33c54a7c9..ae8147f97 100644 --- a/src/syscalls/business_logic_syscall_handler.rs +++ b/src/syscalls/business_logic_syscall_handler.rs @@ -536,7 +536,7 @@ impl<'a, S: StateReader, C: ContractClassCache> BusinessLogicSyscallHandler<'a, let block_number = request.block_number; let current_block_number = self.block_context.block_info.block_number; - if block_number > current_block_number - 10 { + if current_block_number < 10 || block_number > current_block_number - 10 { let out_of_range_felt = Felt252::from_bytes_be("Block number out of range".as_bytes()); let retdata_start = self.allocate_segment(vm, vec![MaybeRelocatable::from(out_of_range_felt)])?; diff --git a/src/syscalls/native_syscall_handler.rs b/src/syscalls/native_syscall_handler.rs index d68028888..88840e5bc 100644 --- a/src/syscalls/native_syscall_handler.rs +++ b/src/syscalls/native_syscall_handler.rs @@ -87,9 +87,14 @@ impl<'a, 'cache, S: StateReader, C: ContractClassCache> StarkNetSyscallHandler gas: &mut u128, ) -> SyscallResult { tracing::debug!("Called `get_block_hash({block_number})` from Cairo Native"); - self.handle_syscall_request(gas, "get_block_hash")?; + let current_block_number = self.block_context.block_info.block_number; + + if current_block_number < 10 || block_number > current_block_number - 10 { + let out_of_range_felt = Felt252::from_bytes_be("Block number out of range".as_bytes()); + return Err(vec![out_of_range_felt]); + } let key: Felt252 = block_number.into(); let block_hash_address = Address(1.into()); @@ -99,7 +104,7 @@ impl<'a, 'cache, S: StateReader, C: ContractClassCache> StarkNetSyscallHandler .get_storage_at(&(block_hash_address, key.to_be_bytes())) { Ok(value) => Ok(value), - Err(_) => Ok(Felt252::zero()), + Err(e) => Err(vec![Felt252::from_bytes_be(e.to_string().as_bytes())]), } } diff --git a/tests/cairo_native.rs b/tests/cairo_native.rs index c1a90b514..962223b2e 100644 --- a/tests/cairo_native.rs +++ b/tests/cairo_native.rs @@ -188,6 +188,112 @@ fn get_block_hash_test() { assert_eq!(vm_result.l2_to_l1_messages, native_result.l2_to_l1_messages); } +#[test] +#[cfg(feature = "cairo-native")] +fn get_block_hash_test_failure() { + // the result felt in the retdata does not match the 'block out of bound error', + // because it occurs a newer error when unwrapping the result of the syscall. + // we check both errors are the same in vm and native. + use starknet_in_rust::{ + state::contract_class_cache::PermanentContractClassCache, utils::felt_to_hash, + }; + + let sierra_contract_class: cairo_lang_starknet::contract_class::ContractClass = + serde_json::from_str( + std::fs::read_to_string("starknet_programs/cairo2/get_block_hash_basic.sierra") + .unwrap() + .as_str(), + ) + .unwrap(); + + let casm_data = include_bytes!("../starknet_programs/cairo2/get_block_hash_basic.casm"); + let casm_contract_class: CasmContractClass = serde_json::from_slice(casm_data).unwrap(); + + let native_entrypoints = sierra_contract_class.clone().entry_points_by_type; + let native_external_selector = &native_entrypoints.external.get(0).unwrap().selector; + + let casm_entrypoints = casm_contract_class.clone().entry_points_by_type; + let casm_external_selector = &casm_entrypoints.external.get(0).unwrap().selector; + + // Create state reader with class hash data + let contract_class_cache = PermanentContractClassCache::default(); + + let native_class_hash: ClassHash = ClassHash([1; 32]); + let casm_class_hash: ClassHash = ClassHash([2; 32]); + let caller_address = Address(1.into()); + + insert_sierra_class_into_cache( + &contract_class_cache, + native_class_hash, + sierra_contract_class, + ); + + contract_class_cache.set_contract_class( + casm_class_hash, + CompiledClass::Casm(Arc::new(casm_contract_class)), + ); + + let mut state_reader = InMemoryStateReader::default(); + let nonce = Felt252::zero(); + + state_reader + .address_to_class_hash_mut() + .insert(caller_address.clone(), casm_class_hash); + state_reader + .address_to_nonce_mut() + .insert(caller_address.clone(), nonce); + + // Create state from the state_reader and contract cache. + let state_reader = Arc::new(state_reader); + let mut state_vm = + CachedState::new(state_reader.clone(), Arc::new(contract_class_cache.clone())); + + state_vm.cache_mut().storage_initial_values_mut().insert( + (Address(1.into()), felt_to_hash(&Felt252::from(10)).0), + Felt252::from_bytes_be(StarkHash::new([5; 32]).unwrap().bytes()), + ); + let mut state_native = CachedState::new(state_reader, Arc::new(contract_class_cache)); + state_native + .cache_mut() + .storage_initial_values_mut() + .insert( + (Address(1.into()), felt_to_hash(&Felt252::from(10)).0), + Felt252::from_bytes_be(StarkHash::new([5; 32]).unwrap().bytes()), + ); + + // block number (is not inside a valid range) + let calldata = [25.into()].to_vec(); + + let native_result = execute( + &mut state_native, + &caller_address, + &caller_address, + native_external_selector, + &calldata, + EntryPointType::External, + &native_class_hash, + ); + + let vm_result = execute( + &mut state_vm, + &caller_address, + &caller_address, + casm_external_selector, + &calldata, + EntryPointType::External, + &casm_class_hash, + ); + assert_eq!(native_result.calldata, calldata); + assert!(vm_result.failure_flag); + assert!(native_result.failure_flag); + assert_eq!(vm_result.retdata, native_result.retdata); + assert_eq!(native_result.gas_consumed, vm_result.gas_consumed); + assert_eq!( + vm_result.accessed_storage_keys, + native_result.accessed_storage_keys + ); +} + #[test] fn integration_test_erc20() { let sierra_contract_class: cairo_lang_starknet::contract_class::ContractClass =