From d3bc92bd81c52fd07d3cd1653f20a13ed64fd49e Mon Sep 17 00:00:00 2001 From: heytdep Date: Thu, 7 Nov 2024 12:16:40 +0700 Subject: [PATCH] allow caller contract to specify reentry rules --- rust-toolchain.toml | 2 +- soroban-env-common/env.json | 17 +++++++++++-- soroban-env-host/src/host.rs | 14 ++++++++++ soroban-env-host/src/host/frame.rs | 24 +++++++++++++++--- soroban-env-host/src/test/lifecycle.rs | 1 + soroban-env-host/src/vm.rs | 24 +++++++++++++++--- soroban-env-host/src/vm/module_cache.rs | 5 +++- soroban-env-host/src/vm/parsed_module.rs | 14 ++++++++-- .../opt/23/example_reentry_a.wasm | Bin 2244 -> 2269 bytes .../wasm-workspace/reentry_a/src/lib.rs | 13 +++++++--- 10 files changed, 97 insertions(+), 17 deletions(-) diff --git a/rust-toolchain.toml b/rust-toolchain.toml index e340b7641..90ff24e80 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,4 +1,4 @@ [toolchain] -channel = "stable" +channel = "1.81" targets = ["wasm32-unknown-unknown"] components = ["rustc", "cargo", "rustfmt", "clippy", "rust-src"] diff --git a/soroban-env-common/env.json b/soroban-env-common/env.json index 2adfeeedf..d3630dfb6 100644 --- a/soroban-env-common/env.json +++ b/soroban-env-common/env.json @@ -1580,7 +1580,7 @@ ], "return": "Val", "docs": "Calls a function in another contract with arguments contained in vector `args`. If the call is successful, returns the result of the called function. Traps otherwise. This functions enables re-entrancy in the immediate cross-contract call.", - "min_supported_protocol": 22 + "min_supported_protocol": 21 }, { "export": "2", @@ -1601,7 +1601,20 @@ ], "return": "Val", "docs": "Calls a function in another contract with arguments contained in vector `args`, returning either the result of the called function or an `Error` if the called function failed. The returned error is either a custom `ContractError` that the called contract returns explicitly, or an error with type `Context` and code `InvalidAction` in case of any other error in the called contract (such as a host function failure that caused a trap). `try_call` might trap in a few scenarios where the error can't be meaningfully recovered from, such as running out of budget. This functions enables re-entrancy in the immediate cross-contract call.", - "min_supported_protocol": 22 + "min_supported_protocol": 21 + }, + { + "export": "3", + "name": "set_reentrant", + "args": [ + { + "name": "enabled", + "type": "Bool" + } + ], + "return": "Void", + "docs": "Enables the current contract to specify the reentrancy rules.", + "min_supported_protocol": 21 } ] }, diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs index c5680ef7a..8fb7aa8e5 100644 --- a/soroban-env-host/src/host.rs +++ b/soroban-env-host/src/host.rs @@ -166,6 +166,9 @@ struct HostImpl { #[doc(hidden)] #[cfg(any(test, feature = "recording_mode"))] need_to_build_module_cache: RefCell, + + // Enables calling modules that link functions that call with reentry. + enable_reentrant: RefCell, } // Host is a newtype on Rc so we can impl Env for it below. @@ -382,6 +385,8 @@ impl Host { suppress_diagnostic_events: RefCell::new(false), #[cfg(any(test, feature = "recording_mode"))] need_to_build_module_cache: RefCell::new(false), + + enable_reentrant: RefCell::new(false), })) } @@ -2407,6 +2412,15 @@ impl VmCallerEnv for Host { self.try_call_with_params(contract_address, func, args, call_params) } + fn set_reentrant( + &self, + _vmcaller: &mut VmCaller, + enabled: Bool, + ) -> Result { + *self.0.enable_reentrant.borrow_mut() = enabled.try_into()?; + Ok(Void::from(())) + } + // endregion: "call" module functions // region: "buf" module functions diff --git a/soroban-env-host/src/host/frame.rs b/soroban-env-host/src/host/frame.rs index 298e0fab2..8708b54be 100644 --- a/soroban-env-host/src/host/frame.rs +++ b/soroban-env-host/src/host/frame.rs @@ -184,6 +184,10 @@ impl Frame { } impl Host { + pub(crate) fn get_reentrancy_flag(&self) -> Result { + Ok(*self.0.enable_reentrant.borrow()) + } + /// Returns if the host currently has a frame on the stack. /// /// A frame being on the stack usually indicates that a contract is currently @@ -686,7 +690,7 @@ impl Host { let args_vec = args.to_vec(); match &instance.executable { ContractExecutable::Wasm(wasm_hash) => { - let vm = self.instantiate_vm(id, wasm_hash)?; + let vm = self.instantiate_vm(id, wasm_hash, true)?; let relative_objects = Vec::new(); self.with_frame( Frame::ContractVM { @@ -709,7 +713,12 @@ impl Host { } } - fn instantiate_vm(&self, id: &Hash, wasm_hash: &Hash) -> Result, HostError> { + fn instantiate_vm( + &self, + id: &Hash, + wasm_hash: &Hash, + reentry_guard: bool, + ) -> Result, HostError> { #[cfg(any(test, feature = "recording_mode"))] { if !self.in_storage_recording_mode()? { @@ -802,7 +811,14 @@ impl Host { #[cfg(not(any(test, feature = "recording_mode")))] let cost_mode = crate::vm::ModuleParseCostMode::Normal; - Vm::new_with_cost_inputs(self, contract_id, code.as_slice(), costs, cost_mode) + Vm::new_with_cost_inputs( + self, + contract_id, + code.as_slice(), + costs, + cost_mode, + reentry_guard, + ) } pub(crate) fn get_contract_protocol_version( @@ -817,7 +833,7 @@ impl Host { let instance = self.retrieve_contract_instance_from_storage(&storage_key)?; match &instance.executable { ContractExecutable::Wasm(wasm_hash) => { - let vm = self.instantiate_vm(contract_id, wasm_hash)?; + let vm = self.instantiate_vm(contract_id, wasm_hash, false)?; Ok(vm.module.proto_version) } ContractExecutable::StellarAsset => self.get_ledger_protocol_version(), diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs index 2609a70c3..c5e3f56ef 100644 --- a/soroban-env-host/src/test/lifecycle.rs +++ b/soroban-env-host/src/test/lifecycle.rs @@ -2416,6 +2416,7 @@ mod cap_xx_opt_in_reentry { let contract_id_b = host.register_test_contract_wasm(SIMPLE_REENTRY_CONTRACT_B); host.enable_debug().unwrap(); let args = test_vec![&host, contract_id_b].into(); + call_contract(&host, contract_id_a, args); let event_body = ContractEventBody::V0(ContractEventV0 { diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs index 5e7779a8c..b50f6c401 100644 --- a/soroban-env-host/src/vm.rs +++ b/soroban-env-host/src/vm.rs @@ -102,9 +102,20 @@ impl Host { pub(crate) fn make_linker( engine: &wasmi::Engine, symbols: &BTreeSet<(&str, &str)>, + enable_reentrant_linking: bool, ) -> Result, HostError> { let mut linker = Linker::new(&engine); for hf in HOST_FUNCTIONS { + if !enable_reentrant_linking { + if symbols.contains(&("d", "1")) || symbols.contains(&("d", "2")) { + return Err(crate::Error::from_type_and_code( + ScErrorType::WasmVm, + ScErrorCode::ArithDomain, + ) + .try_into()?); + } + } + if symbols.contains(&(hf.mod_str, hf.fn_str)) { (hf.wrap)(&mut linker).map_err(|le| wasmi::Error::Linker(le))?; } @@ -257,7 +268,7 @@ impl Vm { if let Some(linker) = &*host.try_borrow_linker()? { Self::instantiate(host, contract_id, parsed_module, linker) } else { - let linker = parsed_module.make_linker(host)?; + let linker = parsed_module.make_linker(host, true)?; Self::instantiate(host, contract_id, parsed_module, &linker) } } @@ -286,13 +297,16 @@ impl Vm { let cost_inputs = VersionedContractCodeCostInputs::V0 { wasm_bytes: wasm.len(), }; - Self::new_with_cost_inputs( + + let vm = Self::new_with_cost_inputs( host, contract_id, wasm, cost_inputs, ModuleParseCostMode::Normal, - ) + false, + ); + vm } pub(crate) fn new_with_cost_inputs( @@ -301,11 +315,13 @@ impl Vm { wasm: &[u8], cost_inputs: VersionedContractCodeCostInputs, cost_mode: ModuleParseCostMode, + reentry_guard: bool, ) -> Result, HostError> { let _span = tracy_span!("Vm::new"); VmInstantiationTimer::new(host.clone()); let parsed_module = Self::parse_module(host, wasm, cost_inputs, cost_mode)?; - let linker = parsed_module.make_linker(host)?; + let linker = parsed_module.make_linker(host, reentry_guard)?; + Self::instantiate(host, contract_id, parsed_module, &linker) } diff --git a/soroban-env-host/src/vm/module_cache.rs b/soroban-env-host/src/vm/module_cache.rs index e93df15b9..f798ac7b8 100644 --- a/soroban-env-host/src/vm/module_cache.rs +++ b/soroban-env-host/src/vm/module_cache.rs @@ -132,7 +132,10 @@ impl ModuleCache { } pub fn make_linker(&self, host: &Host) -> Result, HostError> { - self.with_import_symbols(host, |symbols| Host::make_linker(&self.engine, symbols)) + let enable_reentrant_linking = host.get_reentrancy_flag()?; + self.with_import_symbols(host, |symbols| { + Host::make_linker(&self.engine, symbols, enable_reentrant_linking) + }) } pub fn get_module( diff --git a/soroban-env-host/src/vm/parsed_module.rs b/soroban-env-host/src/vm/parsed_module.rs index 3857e2f14..85f278983 100644 --- a/soroban-env-host/src/vm/parsed_module.rs +++ b/soroban-env-host/src/vm/parsed_module.rs @@ -193,9 +193,19 @@ impl ParsedModule { callback(&symbols) } - pub fn make_linker(&self, host: &Host) -> Result, HostError> { + pub fn make_linker( + &self, + host: &Host, + reentry_guard: bool, + ) -> Result, HostError> { self.with_import_symbols(host, |symbols| { - Host::make_linker(self.module.engine(), symbols) + let enable_reentrant_linking = if reentry_guard { + host.get_reentrancy_flag()? + } else { + true + }; + + Host::make_linker(self.module.engine(), symbols, enable_reentrant_linking) }) } diff --git a/soroban-test-wasms/wasm-workspace/opt/23/example_reentry_a.wasm b/soroban-test-wasms/wasm-workspace/opt/23/example_reentry_a.wasm index c3815269175dc7c916d0ee0be8d9e84e452b0d34..c1a48c71cc2586de6841cf5aaaa2f9a9b1230bd3 100755 GIT binary patch delta 516 zcmZvXyK7WY5XR>_kInAANtjq=H;SVuLO$*7X&NaO6{Vu5WQd`D8XvIc^Jj_ zbu>Psg@u(^SlU|n7g#A+ihqQ9&So)<&BuK6`_9aq&m-SQn9g0f1R}!8;j`$XTc%P@ z?a%gB&_Oe_JD4TlJaRHv#9Yv8IMCA>vUaTDoTrK%s5&>`eJycG>IRb}t+{eN+g3|D z3$ybJS1(+gpQAKP;_}ya`(db|KT!8Zz@`4iIPPg1$MHxnn+ZJD4HNke-1;v&h-doM zw3|7+g19J#D3~u{UESB&vCk$taTZ znm)8+C)T&Q6f{-XF6o$`x&&=%Z%&J!#myNhrNwJH&Xh^XHQn*{C2t)`5Q!R?14n&Itu@SFd z38Drk$B%q$;*UrjNRcvSN-g{cY3#k@F4)+--1j*T=iJNwKm8HSzI_8C!i(NSn`CVK zY01Yt9yv0Hn1EF*xLk82%{nQ5P-OP}KxMMb8PCgw97dcE!zjl|yE%{z=T}!rf@7ZE)@2xwPesR z@k95r)Kt+CfDLJ}!6&gi_ i64; + #[link_name = "_"] + pub fn call_contract(contract: i64, func: i64, args: i64, ) -> i64; + + #[allow(improper_ctypes)] + #[link_name = "3"] + pub fn set_reentrant(enabled: i64, ) -> i64; } #[contract] @@ -19,9 +23,12 @@ impl Contract { let called_val = called.as_val().get_payload() as i64; let func_val = func.as_val().get_payload() as i64; let args_val = args.as_val().get_payload() as i64; + + let set_reentrant_flag = Val::from_bool(true).as_val().get_payload() as i64; unsafe { - call_reentrant(called_val, func_val, args_val); + set_reentrant(set_reentrant_flag); + call_contract(called_val, func_val, args_val); }; }