From f005eae04a8171a2e55f29c9f06545b5e069a021 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Wed, 19 Aug 2026 12:06:08 +0400 Subject: [PATCH 1/2] fix(engine)!: refuse engine calls from the response allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2442. `handle` writes every engine call's response into WASM memory through the template's own `tari_alloc`, so servicing a call runs template code — while an invocation is in flight, where engine calls are permitted. A `tari_alloc` that calls the engine therefore cycles host -> WASM -> host once per response and exhausts the native stack, aborting the validator on SIGSEGV rather than rejecting the transaction. Nothing bounded it. `max_call_depth` never saw it: no `CallInvoke`, no new `WasmProcess`, one call frame throughout. Metering did not get there first either — a `tari_alloc` body costs on the order of tens of points against a 100M per-call ceiling, millions of rounds past what the stack survives. Only `EmitLog` is count-capped, and ops like `GenerateUniqueId` are not. #2440 closed the two entry points that run outside an invocation, the `CallInfo` allocation and the return-value free. This closes the third by the same means: the invocation window is shut for the duration of the response allocation, so all three places the engine drives template code are now unable to call back into it. A refusal is drained on both paths out of the invocation, not just the trap path. `tari_engine_entrypoint` can only answer a refused call with a null pointer, and a template that ignores it returns normally, so checking only the trap path let the refusal be dropped and the transaction commit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UURir3p3vkw4cS48CDz3UX --- crates/engine/src/wasm/environment.rs | 9 - crates/engine/src/wasm/process.rs | 227 +++++++++--------- .../engine/tests/templates/buggy/Cargo.toml | 1 + .../engine/tests/templates/buggy/src/lib.rs | 39 ++- crates/engine/tests/test.rs | 14 ++ 5 files changed, 165 insertions(+), 125 deletions(-) diff --git a/crates/engine/src/wasm/environment.rs b/crates/engine/src/wasm/environment.rs index b88c6163ba..cfb43e9d42 100644 --- a/crates/engine/src/wasm/environment.rs +++ b/crates/engine/src/wasm/environment.rs @@ -160,15 +160,6 @@ impl WasmEnv { self.last_panic = Some(message); } - pub(super) fn alloc(&self, store: &mut S, len: u32) -> Result, WasmExecutionError> { - let ptr = self.mem_alloc_func()?.call(store, len)?; - if ptr.is_null() { - return Err(WasmExecutionError::MemoryAllocationFailed); - } - - Ok(ptr) - } - /// Hands out the template's `tari_alloc` as an owned handle, for callers that must let go of /// their borrow of this environment before calling it. `tari_alloc` is template code, and /// template code can call `tari_engine`, which takes its own `&mut` to this environment. diff --git a/crates/engine/src/wasm/process.rs b/crates/engine/src/wasm/process.rs index 5206b35258..649aee6ca8 100644 --- a/crates/engine/src/wasm/process.rs +++ b/crates/engine/src/wasm/process.rs @@ -44,18 +44,7 @@ use tari_template_lib::{ }, types::{LogLevel, engine_args::SignatureInvokeArg}, }; -use wasmer::{ - AsStoreMut, - AsStoreRef, - Function, - FunctionEnv, - FunctionEnvMut, - Instance, - Store, - StoreMut, - WasmPtr, - imports, -}; +use wasmer::{AsStoreMut, AsStoreRef, Function, FunctionEnv, FunctionEnvMut, Instance, Store, WasmPtr, imports}; use wasmer_middlewares::metering::{MeteringPoints, get_remaining_points, set_remaining_points}; use crate::{ @@ -244,123 +233,108 @@ impl WasmProcess { return WasmPtr::null(); } - let (env_mut, mut store) = env.data_and_store_mut(); - - // Only a template function invocation may call the engine. The engine also enters WASM to - // run `tari_alloc`/`tari_free`, which happens outside any invocation: an engine call made - // from there would mutate state and emit effects that no invocation is metered or charged - // for. `WasmProcess::alloc_checked`/`free_checked` turn the null returned here into the - // recorded refusal, so a template that ignores the null cannot proceed either. - if !env_mut.is_in_template_invocation() { - env_mut.set_refused_engine_call(op); - return WasmPtr::null(); - } - - // Sync this invocation's in-flight meter consumption onto the transaction total before - // dispatching, so budget and allowance checks made inside the host call (native - // verification pre-charges, nested cross-template call budgets) see it. Without this, a - // call could spend its whole metering allowance and still pass mid-call checks that read - // the stale end-of-invocation total. - if let Some(delta) = env_mut.take_unsynced_in_flight_points(&mut store) && - let Err(err) = env_mut.state_mut().interface_mut().record_wasm_execution(delta) { - env_mut.set_last_engine_error(err); - return WasmPtr::null(); + let (env_mut, mut store) = env.data_and_store_mut(); + + // Only a template function invocation may call the engine. The engine also enters WASM + // to run `tari_alloc`/`tari_free` — staging a `CallInfo`, writing an engine call's + // response, releasing a returned value — and that template code runs outside any + // invocation. `WasmProcess::alloc_checked`/`free_checked` and `Self::handle` turn the + // null returned here into the recorded refusal, so a template that ignores the null + // cannot proceed either. + if !env_mut.is_in_template_invocation() { + env_mut.set_refused_engine_call(op); + return WasmPtr::null(); + } + + // Sync this invocation's in-flight meter consumption onto the transaction total before + // dispatching, so budget and allowance checks made inside the host call (native + // verification pre-charges, nested cross-template call budgets) see it. Without this, a + // call could spend its whole metering allowance and still pass mid-call checks that + // read the stale end-of-invocation total. + if let Some(delta) = env_mut.take_unsynced_in_flight_points(&mut store) && + let Err(err) = env_mut.state_mut().interface_mut().record_wasm_execution(delta) + { + env_mut.set_last_engine_error(err); + return WasmPtr::null(); + } } log::debug!(target: LOG_TARGET, "Engine call: {:?}", op); let result = match op { - EngineOp::EmitLog => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: EmitLogArg| { + EngineOp::EmitLog => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: EmitLogArg| { state.interface_mut().emit_log(arg.level, arg.message) }), - EngineOp::ComponentInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: ComponentInvokeArg| { - state - .interface_mut() - .component_invoke(arg.component_ref, arg.action, arg.args.into()) - }) - }, - EngineOp::ResourceInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: ResourceInvokeArg| { - state - .interface_mut() - .resource_invoke(arg.resource_ref, arg.action, arg.args.into()) - }) - }, - EngineOp::VaultInvoke => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: VaultInvokeArg| { + EngineOp::ComponentInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: ComponentInvokeArg| { + state + .interface_mut() + .component_invoke(arg.component_ref, arg.action, arg.args.into()) + }), + EngineOp::ResourceInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: ResourceInvokeArg| { + state + .interface_mut() + .resource_invoke(arg.resource_ref, arg.action, arg.args.into()) + }), + EngineOp::VaultInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: VaultInvokeArg| { state .interface_mut() .vault_invoke(arg.vault_ref, arg.action, arg.args.into()) }), - EngineOp::BucketInvoke => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: BucketInvokeArg| { + EngineOp::BucketInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: BucketInvokeArg| { state .interface_mut() .bucket_invoke(arg.bucket_ref, arg.action, arg.args.into()) }), EngineOp::NonFungibleInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: NonFungibleInvokeArg| { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: NonFungibleInvokeArg| { state .interface_mut() .non_fungible_invoke(arg.address, arg.action, arg.args.into()) }) }, - EngineOp::GenerateUniqueId => Self::handle(store, env_mut, arg_ptr, arg_len, |state, _arg: ()| { + EngineOp::GenerateUniqueId => Self::handle(&mut env, arg_ptr, arg_len, |state, _arg: ()| { state.interface_mut().generate_uuid() }), - EngineOp::ConsensusInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: ConsensusInvokeArg| { - state.interface_mut().consensus_invoke(arg.action) + EngineOp::ConsensusInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: ConsensusInvokeArg| { + state.interface_mut().consensus_invoke(arg.action) + }), + EngineOp::CallerContextInvoke => { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: CallerContextInvokeArg| { + state.interface_mut().caller_context_invoke(arg.action, arg.args.into()) }) }, - EngineOp::CallerContextInvoke => Self::handle( - store, - env_mut, - arg_ptr, - arg_len, - |state, arg: CallerContextInvokeArg| { - state.interface_mut().caller_context_invoke(arg.action, arg.args.into()) - }, - ), - EngineOp::AddressAllocationInvoke => Self::handle( - store, - env_mut, - arg_ptr, - arg_len, - |state, arg: AddressAllocationInvokeArg| state.interface_mut().allocate_address_invoke(arg), - ), - EngineOp::GenerateRandomInvoke => Self::handle( - store, - env_mut, - arg_ptr, - arg_len, - |state, arg: GenerateRandomInvokeArg| state.interface_mut().generate_random_invoke(arg.action), - ), - EngineOp::EmitEvent => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: EmitEventArg| { + EngineOp::AddressAllocationInvoke => { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: AddressAllocationInvokeArg| { + state.interface_mut().allocate_address_invoke(arg) + }) + }, + EngineOp::GenerateRandomInvoke => { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: GenerateRandomInvokeArg| { + state.interface_mut().generate_random_invoke(arg.action) + }) + }, + EngineOp::EmitEvent => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: EmitEventArg| { state.interface_mut().emit_event(arg.topic, arg.payload) }), - EngineOp::CallInvoke => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: CallInvokeArg| { + EngineOp::CallInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: CallInvokeArg| { state.interface_mut().call_invoke(arg.action, arg.args.into()) }), - EngineOp::ProofInvoke => Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: ProofInvokeArg| { + EngineOp::ProofInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: ProofInvokeArg| { state .interface_mut() .proof_invoke(arg.proof_ref, arg.action, arg.args.into()) }), - EngineOp::BuiltinTemplateInvoke => Self::handle( - store, - env_mut, - arg_ptr, - arg_len, - |state, arg: BuiltinTemplateInvokeArg| state.interface_mut().builtin_template_invoke(arg.action), - ), - EngineOp::SignatureInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: SignatureInvokeArg| { - state.interface_mut().signature_invoke(arg.action, arg.args.into()) + EngineOp::BuiltinTemplateInvoke => { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: BuiltinTemplateInvokeArg| { + state.interface_mut().builtin_template_invoke(arg.action) }) }, + EngineOp::SignatureInvoke => Self::handle(&mut env, arg_ptr, arg_len, |state, arg: SignatureInvokeArg| { + state.interface_mut().signature_invoke(arg.action, arg.args.into()) + }), EngineOp::SpendContextInvoke => { - Self::handle(store, env_mut, arg_ptr, arg_len, |state, arg: SpendContextInvokeArg| { + Self::handle(&mut env, arg_ptr, arg_len, |state, arg: SpendContextInvokeArg| { state.interface_mut().spend_context_invoke(arg.action) }) }, @@ -385,8 +359,7 @@ impl WasmProcess { } pub fn handle( - mut store: StoreMut, - env_mut: &mut WasmEnv, + env: &mut FunctionEnvMut>, arg_ptr: WasmPtr, arg_len: u32, f: fn(&mut Runtime, T) -> Result, @@ -396,24 +369,58 @@ impl WasmProcess { U: tari_bor::Encode<()> + tari_bor::CborLen<()>, WasmExecutionError: From, { - // SAFETY: WasmProcess is not used concurrently and templates are not able to spawn threads - let decoded = unsafe { - env_mut.with_memory_slice(&mut store, arg_ptr, arg_len, |arg| { - decode_exact(arg).map_err(|e| { - log::error!(target: LOG_TARGET, "Failed to decode args for engine call: {}", e); - WasmExecutionError::EngineArgDecodeFailed(e) + let decoded = { + let (env_mut, mut store) = env.data_and_store_mut(); + // SAFETY: WasmProcess is not used concurrently and templates are not able to spawn threads + unsafe { + env_mut.with_memory_slice(&mut store, arg_ptr, arg_len, |arg| { + decode_exact(arg).map_err(|e| { + log::error!(target: LOG_TARGET, "Failed to decode args for engine call: {}", e); + WasmExecutionError::EngineArgDecodeFailed(e) + }) }) - }) - }??; - let resp = f(env_mut.state_mut(), decoded)?; + }?? + }; + let resp = f(env.data_mut().state_mut(), decoded)?; let len = encoded_len(&resp)?; - let ptr = env_mut.alloc(&mut store, len as u32)?; + let ptr = Self::alloc_response(env, len)?; + // Encode response directly into the WASM memory. The WASM code is responsible for freeing it. + let (env_mut, mut store) = env.data_and_store_mut(); let mut writer = env_mut.memory_writer(&mut store, ptr)?; encode_into_writer(&resp, &mut writer)?; Ok(ptr) } + /// Allocates room for an engine call's response through the template's own `tari_alloc`. + /// + /// Servicing an engine call therefore runs template code, which is closed out of the invocation + /// window for the duration: a `tari_alloc` that calls the engine would otherwise cycle + /// host -> WASM -> host once per response and exhaust the native stack. Nothing bounds that + /// cycle — it is one call frame, so `max_call_depth` does not see it, and the per-call metering + /// ceiling permits far more rounds than the stack survives. + /// + /// The environment is left unborrowed across the call, since a refusal is recorded through the + /// engine's own `&mut` to it. + fn alloc_response( + env: &mut FunctionEnvMut>, + len: usize, + ) -> Result, WasmExecutionError> { + let len = u32::try_from(len).map_err(|_| WasmExecutionError::MemoryAllocationTooLarge)?; + let alloc_fn = env.data().mem_alloc_func()?; + + env.data_mut().exit_template_invocation(); + let result = alloc_fn.call(&mut *env, len); + env.data_mut().enter_template_invocation(); + + take_refused_engine_call(env.data_mut())?; + let ptr = result?; + if ptr.is_null() { + return Err(WasmExecutionError::MemoryAllocationFailed); + } + Ok(ptr) + } + /// Determine if the version of the template_lib crate in the WASM is valid. pub fn validate_template_abi_version(template_def: &TemplateDef) -> Result<(), WasmExecutionError> { let template_abi_ver = template_def.abi_version(); @@ -498,6 +505,15 @@ impl Invokable for WasmProcess { .interface_mut() .record_wasm_execution(points_consumed.saturating_sub(already_synced))?; + // A refusal or engine error recorded during the invocation fails the call on both paths. + // `tari_engine_entrypoint` can only answer a refused or failed call with a null pointer, + // and a template is free to ignore that and return normally, so the trap path alone is not + // enough to catch it. + take_refused_engine_call(self.env_mut(store))?; + if let Some(err) = self.env_mut(store).take_last_engine_error() { + return Err(WasmExecutionError::RuntimeError(err)); + } + match res { Ok(return_ptr) => { // Read response from memory @@ -523,9 +539,6 @@ impl Invokable for WasmProcess { }) }, Err(err) => { - if let Some(err) = self.env_mut(store).take_last_engine_error() { - return Err(WasmExecutionError::RuntimeError(err)); - } if let Some(message) = self.env_mut(store).take_last_panic_message() { return Err(WasmExecutionError::Panic { message, diff --git a/crates/engine/tests/templates/buggy/Cargo.toml b/crates/engine/tests/templates/buggy/Cargo.toml index 434497f6de..fe255163b6 100644 --- a/crates/engine/tests/templates/buggy/Cargo.toml +++ b/crates/engine/tests/templates/buggy/Cargo.toml @@ -20,3 +20,4 @@ return_empty_abi = [] unexpected_export_function = [] engine_call_in_alloc = [] engine_call_in_free = [] +engine_call_in_response_alloc = [] diff --git a/crates/engine/tests/templates/buggy/src/lib.rs b/crates/engine/tests/templates/buggy/src/lib.rs index 019a80f51e..18382c1e18 100644 --- a/crates/engine/tests/templates/buggy/src/lib.rs +++ b/crates/engine/tests/templates/buggy/src/lib.rs @@ -24,7 +24,7 @@ // The `engine_call_outside_invocation` variants supply their own `tari_alloc`/`tari_free` pair, so // they must not link `tari_template_abi`: that crate exports `tari_free` under the same // `#[no_mangle]` symbol. -#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free")))] +#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc")))] pub use tari_template_abi::tari_alloc; #[global_allocator] @@ -48,14 +48,15 @@ pub static _ABI_TEMPLATE_DEF: [u8; 4] = [4, 0, 0, 0]; feature = "return_null_abi", feature = "no_template_def", feature = "engine_call_in_alloc", - feature = "engine_call_in_free" + feature = "engine_call_in_free", + feature = "engine_call_in_response_alloc" )))] #[unsafe(no_mangle)] pub static _ABI_TEMPLATE_DEF: [u8; 16] = [ 16, 0, 0, 0, 130, 0, 129, 131, 101, 66, 117, 103, 103, 121, 0, 128, ]; -#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free")))] +#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc")))] #[unsafe(no_mangle)] pub unsafe extern "C" fn Buggy_main(_call_info: *mut u8, _call_info_len: usize) -> *mut u8 { core::ptr::null_mut() @@ -76,17 +77,20 @@ pub extern "C" fn i_shouldnt_be_here() -> *mut u8 { /// A template that re-enters the engine from `tari_alloc` or `tari_free`. /// -/// The engine drives both of those itself — `tari_alloc` to stage the `CallInfo` before the -/// invocation, `tari_free` on the pointer the template function returned after it — so they run -/// template code outside any invocation. An engine call made from either must be refused, and -/// refusing it must fail the transaction even though this template ignores the null pointer it -/// gets back. +/// The engine drives both of those itself, at three points: `tari_alloc` to stage the `CallInfo` +/// before the invocation, `tari_alloc` again to write each engine call's response during it, and +/// `tari_free` on the pointer the template function returned after it. All three run template code +/// the engine called. An engine call from any of them must be refused, and refusing it must fail +/// the transaction even though this template ignores the null pointer it gets back. +/// +/// The response allocation is the dangerous one: left open it cycles host -> WASM -> host once per +/// response and exhausts the native stack. /// /// The memory layout mirrors `tari_template_abi`: an allocation is `[usize length prefix][payload]` /// and the pointer handed to the engine points at the payload. It is reimplemented here rather /// than reused so these variants do not link `tari_template_abi`, whose `tari_free` would collide /// with the one below. -#[cfg(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free"))] +#[cfg(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc"))] mod engine_call_outside_invocation { use std::alloc::{Layout, alloc, dealloc}; @@ -120,9 +124,18 @@ mod engine_call_outside_invocation { pub extern "C" fn tari_alloc(size: usize) -> *mut u8 { #[cfg(feature = "engine_call_in_alloc")] call_engine(); + // Skipping the `CallInfo` allocation, which is refused before the invocation begins, leaves + // the response allocation as the one that runs while an invocation is in flight. + #[cfg(feature = "engine_call_in_response_alloc")] + if unsafe { core::ptr::read_volatile(&raw const IN_INVOCATION) } { + call_engine(); + } internal_alloc(size) } + #[cfg(feature = "engine_call_in_response_alloc")] + static mut IN_INVOCATION: bool = false; + /// # Safety /// `ptr` must point at the payload of an allocation made by [`tari_alloc`]. #[unsafe(no_mangle)] @@ -148,6 +161,14 @@ mod engine_call_outside_invocation { #[unsafe(no_mangle)] pub unsafe extern "C" fn Buggy_main(_call_info: *mut u8, _call_info_len: usize) -> *mut u8 { + #[cfg(feature = "engine_call_in_response_alloc")] + { + unsafe { core::ptr::write_volatile(&raw mut IN_INVOCATION, true) }; + // The response to this call is written through `tari_alloc` above, which calls the + // engine again. + call_engine(); + unsafe { core::ptr::write_volatile(&raw mut IN_INVOCATION, false) }; + } let ptr = internal_alloc(ENCODED_UNIT.len()); unsafe { ptr.copy_from_nonoverlapping(ENCODED_UNIT.as_ptr(), ENCODED_UNIT.len()) }; ptr diff --git a/crates/engine/tests/test.rs b/crates/engine/tests/test.rs index 811a725b50..2c8117527f 100644 --- a/crates/engine/tests/test.rs +++ b/crates/engine/tests/test.rs @@ -287,6 +287,20 @@ fn test_engine_call_in_tari_alloc() { }); } +/// The third place the engine drives template code: `handle` writes every engine-call response +/// through the template's own `tari_alloc`, which runs *during* an invocation. Left open, a +/// `tari_alloc` that calls the engine cycles host -> WASM -> host once per response and aborts the +/// process on native stack exhaustion — `max_call_depth` never sees it (one call frame) and the +/// per-call metering ceiling permits far more rounds than the stack survives. +#[test] +fn test_engine_call_in_response_alloc() { + let reason = execute_buggy_main("engine_call_in_response_alloc"); + + assert_reject_reason(reason, RuntimeError::EngineCallOutsideInvocation { + op: EngineOp::EmitLog, + }); +} + fn execute_buggy_main(feature: &'static str) -> RejectReason { let mut test = TemplateTest::new_builtin_only(); let template_addr = test.compile_new_template( From bf680c193785a94423154bca4f6c75130ae8566c Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Wed, 19 Aug 2026 13:44:46 +0400 Subject: [PATCH 2/2] refactor(engine): make the suspended invocation window restore rather than reopen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review on #2446. `alloc_response` reopened the window unconditionally, which is only correct because `handle` sits behind the entrypoint guard. Capture the previous state and restore it, so the bracket is right wherever it is called from, and make `handle` private — it has no callers outside this module. Order the two checks in `invoke` by what actually happens: a swallowed engine error is the live case, and the refusal drain is a backstop for a window-closing site added later without one. Wrap the feature cfgs in the `buggy` template that exceeded the line limit. Root `cargo fmt --all` does not reach that crate, which declares its own workspace. Rename its `engine_call_outside_invocation` module: one of its variants now runs inside an invocation, with the engine closing the window around it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UURir3p3vkw4cS48CDz3UX --- crates/engine/src/wasm/environment.rs | 13 +++++++++++ crates/engine/src/wasm/process.rs | 18 ++++++++------- .../engine/tests/templates/buggy/src/lib.rs | 22 ++++++++++++++----- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/wasm/environment.rs b/crates/engine/src/wasm/environment.rs index cfb43e9d42..99b1760ac6 100644 --- a/crates/engine/src/wasm/environment.rs +++ b/crates/engine/src/wasm/environment.rs @@ -104,6 +104,19 @@ impl WasmEnv { self.in_template_invocation = false; } + /// Closes the window for a call into template code the engine drives from inside an invocation, + /// returning its previous state. Pair with [`Self::restore_template_invocation`] so the window + /// is put back as it was rather than opened. + pub(super) fn suspend_template_invocation(&mut self) -> bool { + let was_open = self.in_template_invocation; + self.in_template_invocation = false; + was_open + } + + pub(super) fn restore_template_invocation(&mut self, was_open: bool) { + self.in_template_invocation = was_open; + } + pub(super) fn is_in_template_invocation(&self) -> bool { self.in_template_invocation } diff --git a/crates/engine/src/wasm/process.rs b/crates/engine/src/wasm/process.rs index 649aee6ca8..bb61e1f3fb 100644 --- a/crates/engine/src/wasm/process.rs +++ b/crates/engine/src/wasm/process.rs @@ -358,7 +358,7 @@ impl WasmProcess { }) } - pub fn handle( + fn handle( env: &mut FunctionEnvMut>, arg_ptr: WasmPtr, arg_len: u32, @@ -409,9 +409,9 @@ impl WasmProcess { let len = u32::try_from(len).map_err(|_| WasmExecutionError::MemoryAllocationTooLarge)?; let alloc_fn = env.data().mem_alloc_func()?; - env.data_mut().exit_template_invocation(); + let was_open = env.data_mut().suspend_template_invocation(); let result = alloc_fn.call(&mut *env, len); - env.data_mut().enter_template_invocation(); + env.data_mut().restore_template_invocation(was_open); take_refused_engine_call(env.data_mut())?; let ptr = result?; @@ -505,14 +505,16 @@ impl Invokable for WasmProcess { .interface_mut() .record_wasm_execution(points_consumed.saturating_sub(already_synced))?; - // A refusal or engine error recorded during the invocation fails the call on both paths. - // `tari_engine_entrypoint` can only answer a refused or failed call with a null pointer, - // and a template is free to ignore that and return normally, so the trap path alone is not - // enough to catch it. - take_refused_engine_call(self.env_mut(store))?; + // An engine error recorded during the invocation fails the call on both paths. + // `tari_engine_entrypoint` can only answer a failed call with a null pointer, and a + // template is free to ignore that and return normally, so the trap path alone is not enough + // to catch it. if let Some(err) = self.env_mut(store).take_last_engine_error() { return Err(WasmExecutionError::RuntimeError(err)); } + // Every site that closes the window drains its own refusal before returning, so this + // catches only a site that is later added without one. + take_refused_engine_call(self.env_mut(store))?; match res { Ok(return_ptr) => { diff --git a/crates/engine/tests/templates/buggy/src/lib.rs b/crates/engine/tests/templates/buggy/src/lib.rs index 18382c1e18..230db45ce1 100644 --- a/crates/engine/tests/templates/buggy/src/lib.rs +++ b/crates/engine/tests/templates/buggy/src/lib.rs @@ -21,10 +21,14 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #![allow(non_snake_case)] -// The `engine_call_outside_invocation` variants supply their own `tari_alloc`/`tari_free` pair, so +// The `engine_call_in_alloc_or_free` variants supply their own `tari_alloc`/`tari_free` pair, so // they must not link `tari_template_abi`: that crate exports `tari_free` under the same // `#[no_mangle]` symbol. -#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc")))] +#[cfg(not(any( + feature = "engine_call_in_alloc", + feature = "engine_call_in_free", + feature = "engine_call_in_response_alloc" +)))] pub use tari_template_abi::tari_alloc; #[global_allocator] @@ -56,7 +60,11 @@ pub static _ABI_TEMPLATE_DEF: [u8; 16] = [ 16, 0, 0, 0, 130, 0, 129, 131, 101, 66, 117, 103, 103, 121, 0, 128, ]; -#[cfg(not(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc")))] +#[cfg(not(any( + feature = "engine_call_in_alloc", + feature = "engine_call_in_free", + feature = "engine_call_in_response_alloc" +)))] #[unsafe(no_mangle)] pub unsafe extern "C" fn Buggy_main(_call_info: *mut u8, _call_info_len: usize) -> *mut u8 { core::ptr::null_mut() @@ -90,8 +98,12 @@ pub extern "C" fn i_shouldnt_be_here() -> *mut u8 { /// and the pointer handed to the engine points at the payload. It is reimplemented here rather /// than reused so these variants do not link `tari_template_abi`, whose `tari_free` would collide /// with the one below. -#[cfg(any(feature = "engine_call_in_alloc", feature = "engine_call_in_free", feature = "engine_call_in_response_alloc"))] -mod engine_call_outside_invocation { +#[cfg(any( + feature = "engine_call_in_alloc", + feature = "engine_call_in_free", + feature = "engine_call_in_response_alloc" +))] +mod engine_call_in_alloc_or_free { use std::alloc::{Layout, alloc, dealloc}; use super::tari_engine;