From 43443da4324674a2c550c063c5709dc9d539a485 Mon Sep 17 00:00:00 2001 From: Nimrod Weiss Date: Sun, 24 Nov 2024 17:21:20 +0200 Subject: [PATCH] feat(blockifier): disallow contract addresses less than 16 --- .../security_tests_contract_compiled.json | 2 +- .../cairo0/security_tests_contract.cairo | 2 +- .../resources/versioned_constants_0_13_4.json | 4 ++-- .../src/execution/syscalls/syscall_base.rs | 3 +-- .../syscalls/syscall_tests/get_block_hash.rs | 5 ++--- .../transaction/account_transactions_test.rs | 3 +-- .../native_blockifier/src/py_block_executor.rs | 18 ++++++++---------- .../src/py_block_executor_test.rs | 10 +++++++--- crates/starknet_api/src/core.rs | 5 +++++ crates/starknet_api/src/lib.rs | 3 +++ 10 files changed, 31 insertions(+), 24 deletions(-) diff --git a/crates/blockifier/feature_contracts/cairo0/compiled/security_tests_contract_compiled.json b/crates/blockifier/feature_contracts/cairo0/compiled/security_tests_contract_compiled.json index 83d1f222c7..e73ef0d479 100644 --- a/crates/blockifier/feature_contracts/cairo0/compiled/security_tests_contract_compiled.json +++ b/crates/blockifier/feature_contracts/cairo0/compiled/security_tests_contract_compiled.json @@ -1027,7 +1027,7 @@ "0x43616c6c436f6e7472616374", "0x400280007ffd7fff", "0x480680017fff8000", - "0x0", + "0x19", "0x400280017ffd7fff", "0x480680017fff8000", "0x0", diff --git a/crates/blockifier/feature_contracts/cairo0/security_tests_contract.cairo b/crates/blockifier/feature_contracts/cairo0/security_tests_contract.cairo index 8cf8d83cc0..bf98b7587f 100644 --- a/crates/blockifier/feature_contracts/cairo0/security_tests_contract.cairo +++ b/crates/blockifier/feature_contracts/cairo0/security_tests_contract.cairo @@ -230,7 +230,7 @@ func test_bad_call_address{syscall_ptr: felt*}() { func test_bad_syscall_request_arg_type{syscall_ptr: felt*}() { assert syscall_ptr[0] = CALL_CONTRACT_SELECTOR; // Contract address. - assert syscall_ptr[1] = 0; + assert syscall_ptr[1] = 25; // Function selector. assert syscall_ptr[2] = 0; // Calldata size. diff --git a/crates/blockifier/resources/versioned_constants_0_13_4.json b/crates/blockifier/resources/versioned_constants_0_13_4.json index 1bf7c6fd66..bc6309732d 100644 --- a/crates/blockifier/resources/versioned_constants_0_13_4.json +++ b/crates/blockifier/resources/versioned_constants_0_13_4.json @@ -235,10 +235,10 @@ "n_memory_holes": 0 }, "Deploy": { - "n_steps": 1132, + "n_steps": 1158, "builtin_instance_counter": { "pedersen_builtin": 7, - "range_check_builtin": 18 + "range_check_builtin": 22 }, "n_memory_holes": 0 }, diff --git a/crates/blockifier/src/execution/syscalls/syscall_base.rs b/crates/blockifier/src/execution/syscalls/syscall_base.rs index 15ce73bbad..c3e0d9e8d2 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_base.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_base.rs @@ -38,7 +38,6 @@ pub fn get_block_hash_base( } let key = StorageKey::try_from(Felt::from(requested_block_number))?; - let block_hash_contract_address = - ContractAddress::try_from(Felt::from(constants::BLOCK_HASH_CONTRACT_ADDRESS))?; + let block_hash_contract_address = ContractAddress::from(constants::BLOCK_HASH_CONTRACT_ADDRESS); Ok(state.get_storage_at(block_hash_contract_address, key)?) } diff --git a/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs b/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs index 83c7af1a63..bf0dff46af 100644 --- a/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs +++ b/crates/blockifier/src/execution/syscalls/syscall_tests/get_block_hash.rs @@ -7,7 +7,7 @@ use starknet_api::{calldata, felt}; use starknet_types_core::felt::Felt; use test_case::test_case; -use crate::abi::constants; +use crate::abi::constants::{self, BLOCK_HASH_CONTRACT_ADDRESS}; use crate::context::ChainInfo; use crate::execution::call_info::CallExecution; use crate::execution::entry_point::CallEntryPoint; @@ -33,8 +33,7 @@ fn initialize_state(test_contract: FeatureContract) -> (CachedState ChainInfo { ChainInfo::try_from(self).expect("Failed to convert chain info.") } + + pub fn create_for_testing() -> Self { + Self { + chain_id: ChainId::Other("".to_string()), + deprecated_fee_token_address: PyFelt::from(19_u8), + fee_token_address: PyFelt::from(19_u8), + } + } } impl TryFrom for ChainInfo { @@ -474,16 +482,6 @@ impl TryFrom for ChainInfo { } } -impl Default for PyOsConfig { - fn default() -> Self { - Self { - chain_id: ChainId::Other("".to_string()), - deprecated_fee_token_address: Default::default(), - fee_token_address: Default::default(), - } - } -} - fn serialize_failure_reason(error: TransactionExecutorError) -> RawTransactionExecutionResult { // TODO(Yoni, 1/7/2024): re-consider this serialization. serde_json::to_vec(&format!("{}", error)).expect(RESULT_SERIALIZE_ERR) diff --git a/crates/native_blockifier/src/py_block_executor_test.rs b/crates/native_blockifier/src/py_block_executor_test.rs index 1bd931d7c1..28652b4729 100644 --- a/crates/native_blockifier/src/py_block_executor_test.rs +++ b/crates/native_blockifier/src/py_block_executor_test.rs @@ -39,7 +39,7 @@ fn global_contract_cache_update() { let temp_storage_path = tempfile::tempdir().unwrap().into_path(); let mut block_executor = PyBlockExecutor::create_for_testing( PyConcurrencyConfig::default(), - PyOsConfig::default(), + PyOsConfig::create_for_testing(), temp_storage_path, 4000, ); @@ -63,7 +63,11 @@ fn global_contract_cache_update() { let sentinel_block_number_and_hash = None; // Information does not exist for block 0. block_executor .setup_block_execution( - PyBlockInfo { block_number: 1, ..PyBlockInfo::default() }, + PyBlockInfo { + block_number: 1, + sequencer_address: PyFelt::from(19_u8), + ..PyBlockInfo::default() + }, sentinel_block_number_and_hash, ) .unwrap(); @@ -119,7 +123,7 @@ fn global_contract_cache_update_large_contract() { let temp_storage_path = tempfile::tempdir().unwrap().into_path(); let mut block_executor = PyBlockExecutor::native_create_for_testing( Default::default(), - Default::default(), + PyOsConfig::create_for_testing(), temp_storage_path, 4000, ); diff --git a/crates/starknet_api/src/core.rs b/crates/starknet_api/src/core.rs index 4841e01a91..fafb3c3162 100644 --- a/crates/starknet_api/src/core.rs +++ b/crates/starknet_api/src/core.rs @@ -138,6 +138,8 @@ impl From for ContractAddress { impl_from_through_intermediate!(u128, ContractAddress, u8, u16, u32, u64); +/// The maximal contract address of reserved contracts. +pub const MAX_RESERVED_CONTRACT_ADDRESS: u128 = 15; /// The maximal size of storage var. pub const MAX_STORAGE_ITEM_SIZE: u16 = 256; /// The prefix used in the calculation of a contract address. @@ -152,6 +154,9 @@ pub static L2_ADDRESS_UPPER_BOUND: LazyLock = LazyLock::new(|| { impl TryFrom for ContractAddress { type Error = StarknetApiError; fn try_from(hash: StarkHash) -> Result { + if hash <= MAX_RESERVED_CONTRACT_ADDRESS.into() { + return Err(Self::Error::ReservedContractAddress(hash)); + } Ok(Self(PatriciaKey::try_from(hash)?)) } } diff --git a/crates/starknet_api/src/lib.rs b/crates/starknet_api/src/lib.rs index 2531709e67..b803637007 100644 --- a/crates/starknet_api/src/lib.rs +++ b/crates/starknet_api/src/lib.rs @@ -25,6 +25,7 @@ pub mod type_utils; use std::num::ParseIntError; +use hash::StarkHash; use serde_utils::InnerDeserializationError; use crate::transaction::TransactionVersion; @@ -65,6 +66,8 @@ pub enum StarknetApiError { version {cairo_version:?}.", **declare_version )] ContractClassVersionMismatch { declare_version: TransactionVersion, cairo_version: u64 }, + #[error("Invalid contract address, must be < 16, as these addresses are reserved. Got {0}")] + ReservedContractAddress(StarkHash), } pub type StarknetApiResult = Result;