From 459dab6fab992433f931b9f0779b4835eb6695d2 Mon Sep 17 00:00:00 2001 From: dancoombs Date: Tue, 2 Jul 2024 14:19:45 -0500 Subject: [PATCH] feat(sim): ban access to arbitrum stylus contracts during sim --- crates/pool/proto/op_pool/op_pool.proto | 6 ++ crates/pool/src/server/remote/error.rs | 32 ++++++-- crates/rpc/src/eth/error.rs | 1 + crates/sim/src/simulation/context.rs | 10 ++- crates/sim/src/simulation/simulator.rs | 77 +++++++++++++++++-- crates/sim/src/simulation/v0_6/context.rs | 34 ++++++-- crates/sim/src/simulation/v0_7/context.rs | 10 +-- crates/sim/src/simulation/v0_7/tracer.rs | 12 +-- crates/sim/tracer/src/validationTracerV0_6.ts | 20 +++-- crates/sim/tracer/src/validationTracerV0_7.ts | 22 ++++-- crates/types/src/pool/error.rs | 3 + 11 files changed, 178 insertions(+), 49 deletions(-) diff --git a/crates/pool/proto/op_pool/op_pool.proto b/crates/pool/proto/op_pool/op_pool.proto index 0b1e6ab65..32d1dfa54 100644 --- a/crates/pool/proto/op_pool/op_pool.proto +++ b/crates/pool/proto/op_pool/op_pool.proto @@ -649,6 +649,7 @@ message SimulationViolationError { InvalidPaymasterSignature invalid_paymaster_signature = 22; AssociatedStorageDuringDeploy associated_storage_during_deploy = 23; InvalidTimeRange invalid_time_range = 24; + AccessedUnsupportedContractType accessed_unsupported_contract_type = 25; } } @@ -764,3 +765,8 @@ message OperationRevert { message UnknownRevert { bytes revert_bytes = 1; } + +message AccessedUnsupportedContractType { + string contract_type = 1; + bytes contract_address = 2; +} diff --git a/crates/pool/src/server/remote/error.rs b/crates/pool/src/server/remote/error.rs index 33ae98209..4a616ddf2 100644 --- a/crates/pool/src/server/remote/error.rs +++ b/crates/pool/src/server/remote/error.rs @@ -22,13 +22,13 @@ use rundler_types::{ use super::protos::{ mempool_error, precheck_violation_error, simulation_violation_error, validation_revert, - AccessedUndeployedContract, AggregatorValidationFailed, AssociatedStorageDuringDeploy, - AssociatedStorageIsAlternateSender, CallGasLimitTooLow, CallHadValue, - CalledBannedEntryPointMethod, CodeHashChanged, DidNotRevert, DiscardedOnInsertError, Entity, - EntityThrottledError, EntityType, EntryPointRevert, ExistingSenderWithInitCode, - FactoryCalledCreate2Twice, FactoryIsNotContract, InvalidAccountSignature, - InvalidPaymasterSignature, InvalidSignature, InvalidStorageAccess, InvalidTimeRange, - MaxFeePerGasTooLow, MaxOperationsReachedError, MaxPriorityFeePerGasTooLow, + AccessedUndeployedContract, AccessedUnsupportedContractType, AggregatorValidationFailed, + AssociatedStorageDuringDeploy, AssociatedStorageIsAlternateSender, CallGasLimitTooLow, + CallHadValue, CalledBannedEntryPointMethod, CodeHashChanged, DidNotRevert, + DiscardedOnInsertError, Entity, EntityThrottledError, EntityType, EntryPointRevert, + ExistingSenderWithInitCode, FactoryCalledCreate2Twice, FactoryIsNotContract, + InvalidAccountSignature, InvalidPaymasterSignature, InvalidSignature, InvalidStorageAccess, + InvalidTimeRange, MaxFeePerGasTooLow, MaxOperationsReachedError, MaxPriorityFeePerGasTooLow, MempoolError as ProtoMempoolError, MultipleRolesViolation, NotStaked, OperationAlreadyKnownError, OperationDropTooSoon, OperationRevert, OutOfGas, PaymasterBalanceTooLow, PaymasterDepositTooLow, PaymasterIsNotContract, @@ -640,6 +640,18 @@ impl From for ProtoSimulationViolationError { ), } } + SimulationViolation::AccessedUnsupportedContractType(contract_type, address) => { + ProtoSimulationViolationError { + violation: Some( + simulation_violation_error::Violation::AccessedUnsupportedContractType( + AccessedUnsupportedContractType { + contract_type, + contract_address: address.to_proto_bytes(), + }, + ), + ), + } + } } } } @@ -800,6 +812,12 @@ impl TryFrom for SimulationViolation { from_bytes(&e.needed)?, ) } + Some(simulation_violation_error::Violation::AccessedUnsupportedContractType(e)) => { + SimulationViolation::AccessedUnsupportedContractType( + e.contract_type, + from_bytes(&e.contract_address)?, + ) + } None => { bail!("unknown proto mempool simulation violation") } diff --git a/crates/rpc/src/eth/error.rs b/crates/rpc/src/eth/error.rs index b180ea3f5..183a139e9 100644 --- a/crates/rpc/src/eth/error.rs +++ b/crates/rpc/src/eth/error.rs @@ -340,6 +340,7 @@ impl From for EthRpcError { } SimulationViolation::UsedForbiddenPrecompile(_, _, _) | SimulationViolation::AccessedUndeployedContract(_, _) + | SimulationViolation::AccessedUnsupportedContractType(_, _) | SimulationViolation::CalledBannedEntryPointMethod(_) | SimulationViolation::CallHadValue(_) => Self::OpcodeViolationMap(value), SimulationViolation::FactoryCalledCreate2Twice(_) => { diff --git a/crates/sim/src/simulation/context.rs b/crates/sim/src/simulation/context.rs index d739a3199..87600ef54 100644 --- a/crates/sim/src/simulation/context.rs +++ b/crates/sim/src/simulation/context.rs @@ -41,7 +41,7 @@ pub struct ValidationContext { pub(crate) struct TracerOutput { pub(crate) phases: Vec, pub(crate) revert_data: Option, - pub(crate) accessed_contract_addresses: Vec
, + pub(crate) accessed_contracts: HashMap, pub(crate) associated_slots_by_address: AssociatedSlotsByAddress, pub(crate) factory_called_create2_twice: bool, pub(crate) expected_storage: ExpectedStorage, @@ -60,6 +60,14 @@ pub(crate) struct Phase { pub(crate) ext_code_access_info: HashMap, } +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct ContractInfo { + pub(crate) header: String, + pub(crate) opcode: Opcode, + pub(crate) length: u64, +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub(crate) struct AccessInfo { // slot value, just prior this current operation diff --git a/crates/sim/src/simulation/simulator.rs b/crates/sim/src/simulation/simulator.rs index 9baeec2e2..49f1a6de3 100644 --- a/crates/sim/src/simulation/simulator.rs +++ b/crates/sim/src/simulation/simulator.rs @@ -14,7 +14,6 @@ use std::{ collections::{HashMap, HashSet}, marker::PhantomData, - mem, ops::Deref, sync::Arc, }; @@ -351,6 +350,16 @@ where } } + for (address, contract_info) in &tracer_out.accessed_contracts { + if contract_info.header.as_str() == "0xEFF000" { + // All arbitrum stylus contracts start with 0xEFF000 + violations.push(SimulationViolation::AccessedUnsupportedContractType( + "Arbitrum Stylus".to_string(), + *address, + )); + } + } + if tracer_out.factory_called_create2_twice { let factory = entity_infos.get(EntityType::Factory); match factory { @@ -402,7 +411,7 @@ where let aggregator_address = entry_point_out.aggregator_info.map(|info| info.address); let code_hash_future = utils::get_code_hash( self.provider.deref(), - mem::take(&mut tracer_out.accessed_contract_addresses), + tracer_out.accessed_contracts.keys().cloned().collect(), Some(block_id), ); let aggregator_signature_future = self.validate_aggregator_signature( @@ -668,6 +677,7 @@ fn override_infos_staked(eis: &mut EntityInfos, allow_unstaked_addresses: &HashS mod tests { use std::str::FromStr; + use context::ContractInfo; use ethers::types::{Address, BlockId, BlockNumber, Bytes, U256, U64}; use rundler_provider::{AggregatorOut, MockEntryPointV0_6, MockProvider}; use rundler_types::{ @@ -709,11 +719,32 @@ mod tests { fn get_test_context() -> ValidationContext { let tracer_out = TracerOutput { - accessed_contract_addresses: vec![ - Address::from_str("0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789").unwrap(), - Address::from_str("0xb856dbd4fa1a79a46d426f537455e7d3e79ab7c4").unwrap(), - Address::from_str("0x8abb13360b87be5eeb1b98647a016add927a136c").unwrap(), - ], + accessed_contracts: HashMap::from([ + ( + Address::from_str("0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ( + Address::from_str("0xb856dbd4fa1a79a46d426f537455e7d3e79ab7c4").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ( + Address::from_str("0x8abb13360b87be5eeb1b98647a016add927a136c").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ]), associated_slots_by_address: serde_json::from_str(r#" { "0x0000000000000000000000000000000000000000": [ @@ -1148,4 +1179,36 @@ mod tests { let res = simulator.gather_context_violations(&mut context); assert!(res.unwrap().is_empty()); } + + #[tokio::test] + async fn test_accessed_unsupported_contract() { + let (provider, mut ep, mut context_provider) = create_base_config(); + ep.expect_address() + .returning(|| Address::from_str("0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789").unwrap()); + context_provider + .expect_get_specific_violations() + .return_const(vec![]); + + let addr = Address::random(); + let mut context = get_test_context(); + context.tracer_out.accessed_contracts.insert( + addr, + ContractInfo { + header: "0xEFF000".to_string(), + opcode: Opcode::CALL, + length: 32, + }, + ); + + let simulator = create_simulator(provider, ep, context_provider); + let res = simulator.gather_context_violations(&mut context); + + assert_eq!( + res.unwrap(), + vec![SimulationViolation::AccessedUnsupportedContractType( + "Arbitrum Stylus".to_string(), + addr + )] + ); + } } diff --git a/crates/sim/src/simulation/v0_6/context.rs b/crates/sim/src/simulation/v0_6/context.rs index 3cd621671..d74ddf552 100644 --- a/crates/sim/src/simulation/v0_6/context.rs +++ b/crates/sim/src/simulation/v0_6/context.rs @@ -201,18 +201,40 @@ mod tests { types::{Address, Bytes, U256}, utils::hex, }; - use rundler_types::{contracts::v0_6::i_entry_point::FailedOp, v0_6::UserOperation}; + use rundler_types::{contracts::v0_6::i_entry_point::FailedOp, v0_6::UserOperation, Opcode}; + use sim_context::ContractInfo; use super::*; use crate::simulation::context::{Phase, TracerOutput}; fn get_test_tracer_output() -> TracerOutput { TracerOutput { - accessed_contract_addresses: vec![ - Address::from_str("0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789").unwrap(), - Address::from_str("0xb856dbd4fa1a79a46d426f537455e7d3e79ab7c4").unwrap(), - Address::from_str("0x8abb13360b87be5eeb1b98647a016add927a136c").unwrap(), - ], + accessed_contracts: HashMap::from([ + ( + Address::from_str("0x5ff137d4b0fdcd49dca30c7cf57e578a026d2789").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ( + Address::from_str("0xb856dbd4fa1a79a46d426f537455e7d3e79ab7c4").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ( + Address::from_str("0x8abb13360b87be5eeb1b98647a016add927a136c").unwrap(), + ContractInfo { + header: "0x608060".to_string(), + opcode: Opcode::CALL, + length: 32, + } + ), + ]), associated_slots_by_address: serde_json::from_str(r#" { "0x0000000000000000000000000000000000000000": [ diff --git a/crates/sim/src/simulation/v0_7/context.rs b/crates/sim/src/simulation/v0_7/context.rs index 348f843c2..26ff9cef6 100644 --- a/crates/sim/src/simulation/v0_7/context.rs +++ b/crates/sim/src/simulation/v0_7/context.rs @@ -346,10 +346,10 @@ impl ValidationContextProvider { } // Accessed contracts - let accessed_contract_addresses = tracer_out + let accessed_contracts = tracer_out .calls_from_entry_point .iter() - .flat_map(|call| call.contract_size.keys().cloned()) + .flat_map(|call| call.contract_info.clone()) .collect(); // Associated slots @@ -378,7 +378,7 @@ impl ValidationContextProvider { Ok(ContextTracerOutput { phases, revert_data: None, - accessed_contract_addresses, + accessed_contracts, associated_slots_by_address: AssociatedSlotsByAddress(associated_slots_by_address), factory_called_create2_twice, expected_storage: tracer_out.expected_storage, @@ -417,8 +417,8 @@ impl ValidationContextProvider { let mut forbidden_precompiles_used = vec![]; let mut undeployed_contract_accesses = vec![]; - call.contract_size.iter().for_each(|(address, info)| { - if info.contract_size == 0 { + call.contract_info.iter().for_each(|(address, info)| { + if info.length == 0 { if *address < MAX_PRECOMPILE_ADDRESS { // [OP-062] - banned precompiles // The tracer catches any allowed precompiles and does not add them to this list diff --git a/crates/sim/src/simulation/v0_7/tracer.rs b/crates/sim/src/simulation/v0_7/tracer.rs index 2eb10545c..51aea7c1e 100644 --- a/crates/sim/src/simulation/v0_7/tracer.rs +++ b/crates/sim/src/simulation/v0_7/tracer.rs @@ -22,7 +22,7 @@ use rundler_provider::{Provider, SimulationProvider}; use rundler_types::{v0_7::UserOperation, Opcode}; use serde::Deserialize; -use crate::ExpectedStorage; +use crate::{simulation::context::ContractInfo, ExpectedStorage}; #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -43,7 +43,7 @@ pub(super) struct TopLevelCallInfo { pub(super) top_level_target_address: String, pub(super) opcodes: HashMap, pub(super) access: HashMap, - pub(super) contract_size: HashMap, + pub(super) contract_info: HashMap, pub(super) ext_code_access_info: HashMap, pub(super) oog: Option, } @@ -55,14 +55,6 @@ pub(super) struct AccessInfo { pub(super) writes: HashMap, } -#[derive(Clone, Debug, Deserialize)] -#[serde(rename_all = "camelCase")] -#[allow(unused)] -pub(super) struct ContractSizeInfo { - pub(super) opcode: Opcode, - pub(super) contract_size: u64, -} - #[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase", untagged)] pub(super) enum CallInfo { diff --git a/crates/sim/tracer/src/validationTracerV0_6.ts b/crates/sim/tracer/src/validationTracerV0_6.ts index 09f3f87fa..962178b2b 100644 --- a/crates/sim/tracer/src/validationTracerV0_6.ts +++ b/crates/sim/tracer/src/validationTracerV0_6.ts @@ -21,7 +21,7 @@ declare function toWord(s: string | Bytes): Bytes; interface Output { phases: Phase[]; revertData: string | null; - accessedContractAddresses: string[]; + accessedContracts: Record; associatedSlotsByAddress: Record; factoryCalledCreate2Twice: boolean; expectedStorage: Record>; @@ -51,6 +51,12 @@ interface RelevantStepData { stackEnd: BigInt | null; } +interface ContractInfo { + opcode: string; + length: number; + header: string; +} + type InternalPhase = Omit< Phase, | "forbiddenOpcodesUsed" @@ -123,7 +129,7 @@ type StringSet = Record; const phases: Phase[] = []; let revertData: string | null = null; - const accessedContractAddresses: StringSet = {}; + const accessedContracts: Record = {}; const associatedSlotsByAddressMap: Record = {}; const allStorageAccesses: Record> = {}; let factoryCreate2Count = 0; @@ -233,7 +239,7 @@ type StringSet = Record; return { phases, revertData, - accessedContractAddresses: Object.keys(accessedContractAddresses), + accessedContracts, associatedSlotsByAddress, factoryCalledCreate2Twice: factoryCreate2Count > 1, expectedStorage, @@ -371,7 +377,7 @@ type StringSet = Record; const addressHex = toHex(address); if (!isPrecompiled(address) && !PRECOMPILE_WHITELIST[addressHex]) { if ( - !accessedContractAddresses[addressHex] || + !accessedContracts[addressHex] || currentPhase.undeployedContractAccesses[addressHex] ) { // The spec says validation must not access code of undeployed @@ -386,7 +392,11 @@ type StringSet = Record; delete currentPhase.undeployedContractAccesses[addressHex]; } } - accessedContractAddresses[addressHex] = true; + accessedContracts[addressHex] = { + header: toHex(db.getCode(address).subarray(0, 3)), + opcode, + length: db.getCode(address).length, + }; } else if (!PRECOMPILE_WHITELIST[addressHex]) { currentPhase.forbiddenPrecompilesUsed[ getContractCombinedKey(log, addressHex) diff --git a/crates/sim/tracer/src/validationTracerV0_7.ts b/crates/sim/tracer/src/validationTracerV0_7.ts index f12220e8c..66a124d15 100644 --- a/crates/sim/tracer/src/validationTracerV0_7.ts +++ b/crates/sim/tracer/src/validationTracerV0_7.ts @@ -85,18 +85,23 @@ export interface TopLevelCallInfo { topLevelTargetAddress: string opcodes: { [opcode: string]: number } access: { [address: string]: AccessInfo } - contractSize: { [addr: string]: ContractSizeInfo } + contractInfo: { [addr: string]: ContractInfo } extCodeAccessInfo: { [addr: string]: string } oog?: boolean } /** + * Contract info + * * It is illegal to access contracts with no code in validation even if it gets deployed later. * This means we need to store the {@link contractSize} of accessed addresses at the time of access. + * + * Capture the "header" of the contract code for validation. */ -export interface ContractSizeInfo { +export interface ContractInfo { opcode: string - contractSize: number + length: number + header: string } export interface AccessInfo { @@ -301,7 +306,7 @@ interface BundlerCollectorTracer extends LogTracer, Bundler access: {}, opcodes: {}, extCodeAccessInfo: {}, - contractSize: {} + contractInfo: {} } this.topLevelCallCounter++ } else if (opcode === 'LOG1') { @@ -347,10 +352,11 @@ interface BundlerCollectorTracer extends LogTracer, Bundler const addr = toAddress(log.stack.peek(idx).toString(16)) const addrHex = toHex(addr) // this.debug.push('op=' + opcode + ' last=' + this.lastOp + ' stacksize=' + log.stack.length() + ' addr=' + addrHex) - if (this.currentLevel.contractSize[addrHex] == null && !isAllowedPrecompiled(addr)) { - this.currentLevel.contractSize[addrHex] = { - contractSize: db.getCode(addr).length, - opcode + if (this.currentLevel.contractInfo[addrHex] == null && !isAllowedPrecompiled(addr)) { + this.currentLevel.contractInfo[addrHex] = { + length: db.getCode(addr).length, + opcode, + header: toHex(db.getCode(addr).subarray(0, 3)) } } } diff --git a/crates/types/src/pool/error.rs b/crates/types/src/pool/error.rs index e81603251..1defbf47e 100644 --- a/crates/types/src/pool/error.rs +++ b/crates/types/src/pool/error.rs @@ -225,6 +225,9 @@ pub enum SimulationViolation { /// Verification gas limit doesn't have the required buffer on the measured gas #[display("verification gas limit doesn't have the required buffer on the measured gas, limit: {0}, needed: {1}")] VerificationGasLimitBufferTooLow(U256, U256), + /// Unsupported contract type + #[display("accessed unsupported contract type: {0:?} at {1:?}. Address must be whitelisted")] + AccessedUnsupportedContractType(String, Address), } /// Information about a storage violation based on stake status