diff --git a/crates/engine/src/runtime/error.rs b/crates/engine/src/runtime/error.rs index abb5de852d..76665c894e 100644 --- a/crates/engine/src/runtime/error.rs +++ b/crates/engine/src/runtime/error.rs @@ -38,6 +38,7 @@ use tari_ootle_transaction::{ NftCheck, args::{WorkspaceId, WorkspaceOffsetId}, }; +use tari_template_abi::EngineOp; use tari_template_lib::{ args::{CallAction, VaultFreezeFlag}, models::{AddressAllocationId, BucketId, ProofId}, @@ -309,6 +310,8 @@ pub enum RuntimeError { MaxCallDepthExceeded { max_depth: usize }, #[error("{action} can only be called from within a component context")] NotInComponentContext { action: ActionIdent }, + #[error("Engine call '{op}' is only permitted from within a template function invocation")] + EngineCallOutsideInvocation { op: EngineOp }, #[error("Duplicate bucket {bucket_id}")] DuplicateBucket { bucket_id: BucketId }, #[error("Duplicate proof {proof_id}")] diff --git a/crates/engine/src/wasm/environment.rs b/crates/engine/src/wasm/environment.rs index dd85fb0f0d..b88c6163ba 100644 --- a/crates/engine/src/wasm/environment.rs +++ b/crates/engine/src/wasm/environment.rs @@ -20,12 +20,9 @@ // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -use std::{ - fmt::{Debug, Formatter}, - sync::{Arc, Mutex, MutexGuard}, -}; +use std::fmt::{Debug, Formatter}; -use tari_template_abi::{ABI_TEMPLATE_DEF_GLOBAL_NAME, TemplateDef, WASM_PTR_SIZE}; +use tari_template_abi::{ABI_TEMPLATE_DEF_GLOBAL_NAME, EngineOp, TemplateDef, WASM_PTR_SIZE}; use wasmer::{ AsStoreMut, AsStoreRef, @@ -46,15 +43,19 @@ use crate::{ pub(crate) type WasmAllocFn = TypedFunction>; pub(crate) type WasmFreeFn = TypedFunction, ()>; -#[derive(Clone)] +/// State shared between the host and one WASM instance. It lives in the instance's +/// [`wasmer::FunctionEnv`], which hands out `&mut` to whoever holds the store, so nothing here +/// needs interior mutability: the engine executes one instance at a time on one thread. pub struct WasmEnv { memory: Option, state: T, mem_alloc: Option, mem_free: Option, - last_panic: Arc>>, - last_engine_error: Arc>>, - invocation_meter: Arc>>, + last_panic: Option, + last_engine_error: Option, + invocation_meter: Option, + in_template_invocation: bool, + refused_engine_call: Option, } /// Per-invocation view of the Wasmer meter, letting host calls read the in-flight consumption of @@ -74,17 +75,56 @@ impl WasmEnv { state, mem_alloc: None, mem_free: None, - last_panic: Arc::new(Mutex::new(None)), - last_engine_error: Arc::new(Mutex::new(None)), - invocation_meter: Arc::new(Mutex::new(None)), + last_panic: None, + last_engine_error: None, + invocation_meter: None, + in_template_invocation: false, + refused_engine_call: None, } } + /// Marks template code as running inside a function invocation, which is the only context + /// permitted to call the engine. + /// + /// This is deliberately its own state rather than a reading of [`Self::invocation_meter`]. The + /// metering window is a billing concern and may legitimately be widened — for instance to + /// charge the `tari_alloc`/`tari_free` the engine drives around a call — whereas this window + /// must stay closed around the template function itself. Deriving one from the other would let + /// such a change silently re-admit engine calls from `tari_alloc`/`tari_free`. + /// + /// One invocation is in flight per process at a time; cross-template calls run in their own + /// process with their own [`WasmEnv`]. + pub(super) fn enter_template_invocation(&mut self) { + self.in_template_invocation = true; + } + + /// Marks the template function invocation as finished. Template code the engine drives after + /// this point — `tari_free` on the returned pointer — may no longer call the engine. + pub(super) fn exit_template_invocation(&mut self) { + self.in_template_invocation = false; + } + + pub(super) fn is_in_template_invocation(&self) -> bool { + self.in_template_invocation + } + + /// Records that an engine call was refused for being made outside a template function + /// invocation. Kept apart from [`Self::last_engine_error`], which carries failures of calls + /// that were dispatched: a refusal must fail the transaction even when the template ignores + /// the null pointer it is handed, so the host reads it back unambiguously. + pub(super) fn set_refused_engine_call(&mut self, op: EngineOp) { + self.refused_engine_call = Some(op); + } + + pub(super) fn take_refused_engine_call(&mut self) -> Option { + self.refused_engine_call.take() + } + /// Begins metering an invocation that starts with `start_points` on the Wasmer meter. One /// invocation is in flight per process instance at a time (cross-template calls run in their /// own process, with their own meter). - pub(super) fn begin_metered_invocation(&self, instance: Instance, start_points: u64) { - *self.invocation_meter_mut() = Some(InvocationMeter { + pub(super) fn begin_metered_invocation(&mut self, instance: Instance, start_points: u64) { + self.invocation_meter = Some(InvocationMeter { instance, start_points, synced: 0, @@ -93,18 +133,17 @@ impl WasmEnv { /// Ends the in-flight invocation, returning the points already synced to the transaction /// total, so the caller records only the unsynced tail. - pub(super) fn end_metered_invocation(&self) -> u64 { - self.invocation_meter_mut().take().map(|m| m.synced).unwrap_or(0) + pub(super) fn end_metered_invocation(&mut self) -> u64 { + self.invocation_meter.take().map(|m| m.synced).unwrap_or(0) } /// Reads the in-flight invocation's consumed-but-unsynced points from the Wasmer meter and /// marks them synced. Returns `None` when no invocation is in flight (host calls made outside /// a WASM invocation) or nothing new was consumed. - pub(super) fn take_unsynced_in_flight_points(&self, store: &mut S) -> Option { + pub(super) fn take_unsynced_in_flight_points(&mut self, store: &mut S) -> Option { use wasmer_middlewares::metering::{MeteringPoints, get_remaining_points}; - let mut guard = self.invocation_meter_mut(); - let meter = guard.as_mut()?; + let meter = self.invocation_meter.as_mut()?; let consumed = match get_remaining_points(store, &meter.instance) { MeteringPoints::Remaining(n) => meter.start_points.saturating_sub(n), MeteringPoints::Exhausted => meter.start_points, @@ -117,50 +156,46 @@ impl WasmEnv { Some(delta) } - fn invocation_meter_mut(&self) -> MutexGuard<'_, Option> { - self.invocation_meter.lock().expect("invocation_meter poisoned") - } - - pub(super) fn set_last_panic(&self, message: String) { - *self.last_panic_mut() = Some(message); + pub(super) fn set_last_panic(&mut self, message: String) { + self.last_panic = Some(message); } pub(super) fn alloc(&self, store: &mut S, len: u32) -> Result, WasmExecutionError> { - let ptr = self.get_mem_alloc_func()?.call(store, len)?; - if ptr.offset() == 0 { + let ptr = self.mem_alloc_func()?.call(store, len)?; + if ptr.is_null() { return Err(WasmExecutionError::MemoryAllocationFailed); } Ok(ptr) } - pub(super) fn free(&self, store: &mut S, ptr: WasmPtr) -> Result<(), WasmExecutionError> { - let mem_free = self - .mem_free - .as_ref() - .ok_or_else(|| WasmExecutionError::MissingAbiFunction { function: "tari_free" })?; - mem_free.call(store, ptr)?; - Ok(()) - } - - fn last_panic_mut(&self) -> MutexGuard<'_, Option> { - self.last_panic.lock().expect("last_panic poisoned") + /// 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. + pub(super) fn mem_alloc_func(&self) -> Result { + self.mem_alloc + .clone() + .ok_or(WasmExecutionError::MissingAbiFunction { function: "tari_alloc" }) } - pub(super) fn take_last_panic_message(&self) -> Option { - self.last_panic_mut().take() + /// Hands out the template's `tari_free` as an owned handle, under the same borrowing rule as + /// [`Self::mem_alloc_func`]. + pub(super) fn mem_free_func(&self) -> Result { + self.mem_free + .clone() + .ok_or(WasmExecutionError::MissingAbiFunction { function: "tari_free" }) } - fn last_engine_error_mut(&self) -> MutexGuard<'_, Option> { - self.last_engine_error.lock().expect("last_engine_error poisoned") + pub(super) fn take_last_panic_message(&mut self) -> Option { + self.last_panic.take() } - pub(super) fn set_last_engine_error(&self, error: RuntimeError) { - *self.last_engine_error_mut() = Some(error); + pub(super) fn set_last_engine_error(&mut self, error: RuntimeError) { + self.last_engine_error = Some(error); } - pub(super) fn take_last_engine_error(&self) -> Option { - self.last_engine_error_mut().take() + pub(super) fn take_last_engine_error(&mut self) -> Option { + self.last_engine_error.take() } pub(super) fn load_template_def( @@ -292,12 +327,6 @@ impl WasmEnv { &mut self.state } - fn get_mem_alloc_func(&self) -> Result<&TypedFunction>, WasmExecutionError> { - self.mem_alloc - .as_ref() - .ok_or_else(|| WasmExecutionError::MissingAbiFunction { function: "tari_alloc" }) - } - fn get_memory(&self) -> Result<&Memory, WasmExecutionError> { let memory = self.memory.as_ref().ok_or_else(|| WasmExecutionError::MemoryNotSet)?; Ok(memory) diff --git a/crates/engine/src/wasm/process.rs b/crates/engine/src/wasm/process.rs index 0f00a8a598..5206b35258 100644 --- a/crates/engine/src/wasm/process.rs +++ b/crates/engine/src/wasm/process.rs @@ -44,11 +44,22 @@ use tari_template_lib::{ }, types::{LogLevel, engine_args::SignatureInvokeArg}, }; -use wasmer::{AsStoreMut, Function, FunctionEnv, FunctionEnvMut, Instance, Store, StoreMut, WasmPtr, imports}; +use wasmer::{ + AsStoreMut, + AsStoreRef, + Function, + FunctionEnv, + FunctionEnvMut, + Instance, + Store, + StoreMut, + WasmPtr, + imports, +}; use wasmer_middlewares::metering::{MeteringPoints, get_remaining_points, set_remaining_points}; use crate::{ - runtime::{ComputeFunding, Runtime}, + runtime::{ComputeAllowance, ComputeFunding, Runtime, RuntimeError}, traits::Invokable, wasm::{ LoadedWasmTemplate, @@ -63,14 +74,13 @@ const LOG_TARGET: &str = "tari::ootle::engine::wasm::process"; pub struct WasmProcess { module: LoadedWasmTemplate, - env: WasmEnv, + fn_env: FunctionEnv>, instance: Instance, } impl WasmProcess { pub fn init(store: &mut Store, module: LoadedWasmTemplate, state: Runtime) -> Result { - let mut env = WasmEnv::new(state); - let fn_env = FunctionEnv::new(store, env.clone()); + let fn_env = FunctionEnv::new(store, WasmEnv::new(state)); let tari_engine = Function::new_typed_with_env(store, &fn_env, Self::tari_engine_entrypoint); let imports = imports! { @@ -86,13 +96,14 @@ impl WasmProcess { let tari_free = instance.exports.get_typed_function(store, "tari_free")?; fn_env .as_mut(store) - .set_memory(memory.clone()) - .set_alloc_funcs(tari_alloc.clone(), tari_free.clone()); - - // Also set these for the local copy - env.set_memory(memory).set_alloc_funcs(tari_alloc, tari_free); + .set_memory(memory) + .set_alloc_funcs(tari_alloc, tari_free); - Ok(Self { module, env, instance }) + Ok(Self { + module, + fn_env, + instance, + }) } fn with_alloc_and_mem_writer( @@ -112,14 +123,100 @@ impl WasmProcess { } let len = u32::try_from(alloc_size).map_err(|_| WasmExecutionError::MemoryAllocationTooLarge)?; - let ptr = self.env.alloc(store, len)?; + let ptr = self.alloc_checked(store, len)?; + let mut fn_env = self.env_and_store(store); + let (env, mut store) = fn_env.data_and_store_mut(); + let mut writer = env.memory_writer(&mut store, ptr)?; + callback(&mut writer)?; + + Ok(AllocPtr::new(ptr.offset(), len)) + } + + fn env<'a, S: AsStoreRef>(&self, store: &'a S) -> &'a WasmEnv { + self.fn_env.as_ref(store) + } + + fn env_mut<'a, S: AsStoreMut>(&self, store: &'a mut S) -> &'a mut WasmEnv { + self.fn_env.as_mut(store) + } + + /// Borrows the environment together with a store handle, as host calls receive them. Reading + /// or writing the instance's memory needs both at once. + fn env_and_store<'a, S: AsStoreMut>(&self, store: &'a mut S) -> FunctionEnvMut<'a, WasmEnv> { + self.fn_env.clone().into_mut(store) + } + + /// Calls the template's `tari_alloc`, failing the call if it called the engine. + /// + /// The environment is left unborrowed for the duration of the call. `tari_alloc` is template + /// code, and a template that calls `tari_engine` from it has the engine take its own `&mut` to + /// the same environment to record the refusal — so a borrow held across the call would alias. + fn alloc_checked(&self, store: &mut S, len: u32) -> Result, WasmExecutionError> { + let alloc_fn = self.env(store).mem_alloc_func()?; + let result = alloc_fn.call(store, len); + take_refused_engine_call(self.env_mut(store))?; + let ptr = result?; if ptr.is_null() { return Err(WasmExecutionError::MemoryAllocationFailed); } - let mut writer = self.env.memory_writer(store, ptr)?; - callback(&mut writer)?; + Ok(ptr) + } - Ok(AllocPtr::new(ptr.offset(), len)) + /// Calls the template's `tari_free`, failing the call if it called the engine. Borrows the + /// environment under the same rule as [`Self::alloc_checked`]. + fn free_checked(&self, store: &mut S, ptr: WasmPtr) -> Result<(), WasmExecutionError> { + let free_fn = self.env(store).mem_free_func()?; + let result = free_fn.call(store, ptr); + take_refused_engine_call(self.env_mut(store))?; + result?; + Ok(()) + } + + /// Works out how much compute this invocation may run, and what bounds it. + /// + /// The Wasmer meter starts each store at the per-call ceiling (set when the engine compiles the + /// module, see `wasm::module::create_engine`). Lowering it to what remains of the + /// transaction-wide budget stops a transaction from exceeding + /// `MAX_WASM_POINTS_PER_TRANSACTION` by spreading work across many instructions or nested + /// cross-template calls, each of which would otherwise get a fresh per-call budget. When the + /// budget is already spent the allowance is zero and the call traps out-of-gas on its first + /// metered op. + /// + /// It is capped again by the compute the transaction is authorized to run: the fee intent's + /// flat credit, or past the checkpoint what the fees paid can cover. That bounds the compute an + /// under-paying transaction can extract — it traps out-of-gas once it exhausts the allowance + /// rather than running up to the per-transaction hard cap. The allowance is shared with native + /// verification (which pre-charges its point cost), so it is reduced by the combined + /// consumption; the hard cap bounds WASM work only. + fn metering_allowance(&self, store: &mut Store) -> MeteringAllowance { + let per_call_cap = match get_remaining_points(store, &self.instance) { + MeteringPoints::Remaining(n) => n, + MeteringPoints::Exhausted => 0, + }; + let interface = self.env(store).state().interface(); + let consumed = interface.wasm_points_consumed(); + let native_consumed = interface.native_points_consumed(); + let budget_remaining = limits::MAX_WASM_POINTS_PER_TRANSACTION.saturating_sub(consumed); + let allowance_remaining = interface.compute_allowance().map(|allowance| { + let remaining = allowance + .points + .saturating_sub(consumed.saturating_add(native_consumed)); + (allowance, remaining) + }); + + MeteringAllowance { + consumed, + points_before: match allowance_remaining { + Some((_, remaining)) => per_call_cap.min(budget_remaining).min(remaining), + None => per_call_cap.min(budget_remaining), + }, + // Kept when the allowance — rather than the per-transaction hard cap — is what bounds + // this call, so an out-of-gas trap is reported against whatever authorized it rather + // than as a hit cap. + binding_allowance: allowance_remaining + .filter(|(_, remaining)| *remaining < budget_remaining && *remaining <= per_call_cap) + .map(|(allowance, _)| allowance), + } } #[allow(clippy::too_many_lines)] @@ -149,6 +246,16 @@ impl WasmProcess { 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 @@ -355,50 +462,25 @@ impl Invokable for WasmProcess { Ok(()) })?; - // Cap this invocation's metering allowance to whatever remains of the transaction-wide - // budget. A fresh store starts at the per-call ceiling (set when the engine compiles the - // module, see `wasm::module::create_engine`); lowering it to `budget - already_consumed` - // stops a transaction from exceeding `MAX_WASM_POINTS_PER_TRANSACTION` by spreading work - // across many instructions or nested cross-template calls, each of which would otherwise get - // a fresh per-call budget. When the budget is already spent the allowance is zero and the - // call traps out-of-gas on its first metered op. - let per_call_cap = match get_remaining_points(store, &self.instance) { - MeteringPoints::Remaining(n) => n, - MeteringPoints::Exhausted => 0, - }; - let consumed = self.env.state().interface().wasm_points_consumed(); - let budget_remaining = limits::MAX_WASM_POINTS_PER_TRANSACTION.saturating_sub(consumed); - // Cap further to the compute the transaction is authorized to run: the fee intent's flat - // credit, or past the checkpoint what the fees paid can cover. This bounds the compute an - // under-paying transaction can extract: it traps out-of-gas once it exhausts the allowance - // rather than running up to the per-transaction hard cap. The allowance is shared with - // native verification (which pre-charges its point cost), so it is reduced by the combined - // consumption; the hard cap above bounds WASM work only. - let native_consumed = self.env.state().interface().native_points_consumed(); - let allowance_remaining = self.env.state().interface().compute_allowance().map(|allowance| { - let remaining = allowance - .points - .saturating_sub(consumed.saturating_add(native_consumed)); - (allowance, remaining) - }); - let points_before = match allowance_remaining { - Some((_, remaining)) => per_call_cap.min(budget_remaining).min(remaining), - None => per_call_cap.min(budget_remaining), - }; - // The allowance, kept when it — rather than the per-transaction hard cap — is what bounds - // this call, so an out-of-gas trap here is reported against whatever authorized it rather - // than as a hit cap. - let binding_allowance = allowance_remaining - .filter(|(_, remaining)| *remaining < budget_remaining && *remaining <= per_call_cap) - .map(|(allowance, _)| allowance); + let MeteringAllowance { + consumed, + points_before, + binding_allowance, + } = self.metering_allowance(store); set_remaining_points(store, &self.instance, points_before); // Expose the in-flight meter to host calls: consumption inside this invocation must be // visible to budget/allowance checks made mid-call (native verification pre-charges, // nested cross-template call budgets), not only after the call returns. - self.env.begin_metered_invocation(self.instance.clone(), points_before); - - // Call the contract entrypoint + self.env_mut(store) + .begin_metered_invocation(self.instance.clone(), points_before); + + // Call the contract entrypoint. Engine calls are admitted for exactly this window: the + // `tari_alloc` above and the `tari_free` below run template code too, but outside any + // invocation the engine could meter, charge or attribute effects to. Nothing may return + // early between the two calls below, or the window is left open over the free. + self.env_mut(store).enter_template_invocation(); let res = func.call(store, call_info_ptr.as_wasm_ptr(), call_info_ptr.len()); + self.env_mut(store).exit_template_invocation(); let remaining_after_call = get_remaining_points(store, &self.instance); let exhausted = matches!(remaining_after_call, MeteringPoints::Exhausted); @@ -409,9 +491,9 @@ impl Invokable for WasmProcess { MeteringPoints::Exhausted => points_before, }; // Record only the tail not already synced to the transaction total by mid-call host calls. - let already_synced = self.env.end_metered_invocation(); + let already_synced = self.env_mut(store).end_metered_invocation(); // Charging happens before we return the result so fees are recorded even on failure paths. - self.env + self.env_mut(store) .state_mut() .interface_mut() .record_wasm_execution(points_consumed.saturating_sub(already_synced))?; @@ -421,15 +503,16 @@ impl Invokable for WasmProcess { // Read response from memory // SAFETY: WasmProcess is not used concurrently let value = unsafe { - self.env - .with_memory_embedded_len(store, return_ptr.offset(), IndexedValue::from_raw)?? + let mut fn_env = self.env_and_store(store); + let (env, mut store) = fn_env.data_and_store_mut(); + env.with_memory_embedded_len(&mut store, return_ptr.offset(), IndexedValue::from_raw)?? }; // Free allocated memory containing the result - self.env.free(store, return_ptr)?; + self.free_checked(store, return_ptr)?; - self.env.state().interface().validate_return_value(&value)?; - self.env + self.env(store).state().interface().validate_return_value(&value)?; + self.env_mut(store) .state_mut() .interface_mut() .set_last_instruction_output(value.clone())?; @@ -440,10 +523,10 @@ impl Invokable for WasmProcess { }) }, Err(err) => { - if let Some(err) = self.env.take_last_engine_error() { + if let Some(err) = self.env_mut(store).take_last_engine_error() { return Err(WasmExecutionError::RuntimeError(err)); } - if let Some(message) = self.env.take_last_panic_message() { + if let Some(message) = self.env_mut(store).take_last_panic_message() { return Err(WasmExecutionError::Panic { message, runtime_error: err, @@ -472,6 +555,29 @@ impl Invokable for WasmProcess { } } +/// What one invocation may spend on the Wasmer meter, and what bounds it. +struct MeteringAllowance { + /// WASM points the transaction has consumed before this invocation. + consumed: u64, + /// Points to set on the meter for this invocation. + points_before: u64, + /// Set when the authorized compute, not the per-transaction hard cap, is the binding limit. + binding_allowance: Option, +} + +/// Reports an engine call `tari_engine_entrypoint` refused. It can only signal a refusal by +/// returning a null pointer, which a template is free to ignore, so the recorded refusal is what +/// actually fails the call. It takes precedence over any error the alloc or free itself returned, +/// being the cause of it. +fn take_refused_engine_call(env: &mut WasmEnv) -> Result<(), WasmExecutionError> { + match env.take_refused_engine_call() { + Some(op) => Err(WasmExecutionError::RuntimeError( + RuntimeError::EngineCallOutsideInvocation { op }, + )), + None => Ok(()), + } +} + fn debug_handler(mut env: FunctionEnvMut>, arg_ptr: WasmPtr, arg_len: u32) { const WASM_DEBUG_LOG_TARGET: &str = "tari::ootle::wasm"; let (state, mut store) = env.data_and_store_mut(); @@ -497,40 +603,42 @@ fn on_panic_handler( let (state, mut store) = env.data_and_store_mut(); // SAFETY: There is no way to call this function concurrently - unsafe { - state - .with_memory_slice(&mut store, msg_ptr, msg_len as u32, |msg_bytes| { - if msg_bytes.len() > limits::ENGINE_LIMITS.max_panic_message_size { - let Ok(msg) = str::from_utf8(msg_bytes) else { - error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) ", line, col); - return; - }; - log::error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) {}", line, col, msg); - let limit = limits::ENGINE_LIMITS.max_panic_message_size; - let mut end = limit; - // Ensure we truncate at a char boundary (to avoid a panic when calling truncate) - while end > 0 && !msg.is_char_boundary(end) { - end -= 1; - } - error!(target: LOG_TARGET, "Panic message size limit exceeded: for panic {}", msg); - state.set_last_panic(msg[..end].to_string()); - } else { - let msg = String::from_utf8_lossy(msg_bytes); - log::error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) {}", line, col, msg); - state.set_last_panic(msg.into_owned()); + let panic_message = unsafe { + state.with_memory_slice(&mut store, msg_ptr, msg_len as u32, |msg_bytes| { + if msg_bytes.len() > limits::ENGINE_LIMITS.max_panic_message_size { + let Ok(msg) = str::from_utf8(msg_bytes) else { + error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) ", line, col); + return None; + }; + log::error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) {}", line, col, msg); + let limit = limits::ENGINE_LIMITS.max_panic_message_size; + let mut end = limit; + // Ensure we truncate at a char boundary (to avoid a panic when calling truncate) + while end > 0 && !msg.is_char_boundary(end) { + end -= 1; } - }) - .unwrap_or_else(|err| { - log::error!( - target: WASM_DEBUG_LOG_TARGET, - "📣 PANIC: WASM template panicked but did not provide a valid memory pointer to on_panic \ - callback: {}", - err - ); - state.set_last_panic(format!( - "WASM panicked but did not provide a valid message pointer to on_panic callback: {}", - err - )); - }); + error!(target: LOG_TARGET, "Panic message size limit exceeded: for panic {}", msg); + Some(msg[..end].to_string()) + } else { + let msg = String::from_utf8_lossy(msg_bytes); + log::error!(target: WASM_DEBUG_LOG_TARGET, "📣 PANIC: ({}:{}) {}", line, col, msg); + Some(msg.into_owned()) + } + }) + } + .unwrap_or_else(|err| { + log::error!( + target: WASM_DEBUG_LOG_TARGET, + "📣 PANIC: WASM template panicked but did not provide a valid memory pointer to on_panic callback: {}", + err + ); + Some(format!( + "WASM panicked but did not provide a valid message pointer to on_panic callback: {}", + err + )) + }); + + if let Some(message) = panic_message { + state.set_last_panic(message); } } diff --git a/crates/engine/tests/templates/buggy/Cargo.toml b/crates/engine/tests/templates/buggy/Cargo.toml index 3f2e672659..434497f6de 100644 --- a/crates/engine/tests/templates/buggy/Cargo.toml +++ b/crates/engine/tests/templates/buggy/Cargo.toml @@ -18,3 +18,5 @@ no_template_def = [] return_null_abi = [] return_empty_abi = [] unexpected_export_function = [] +engine_call_in_alloc = [] +engine_call_in_free = [] diff --git a/crates/engine/tests/templates/buggy/src/lib.rs b/crates/engine/tests/templates/buggy/src/lib.rs index dbf7e5f09f..019a80f51e 100644 --- a/crates/engine/tests/templates/buggy/src/lib.rs +++ b/crates/engine/tests/templates/buggy/src/lib.rs @@ -21,8 +21,10 @@ // USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #![allow(non_snake_case)] -use core::ptr; - +// 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")))] pub use tari_template_abi::tari_alloc; #[global_allocator] @@ -44,26 +46,128 @@ pub static _ABI_TEMPLATE_DEF: [u8; 4] = [4, 0, 0, 0]; #[cfg(not(any( feature = "return_empty_abi", feature = "return_null_abi", - feature = "no_template_def" + feature = "no_template_def", + feature = "engine_call_in_alloc", + feature = "engine_call_in_free" )))] #[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")))] #[unsafe(no_mangle)] pub unsafe extern "C" fn Buggy_main(_call_info: *mut u8, _call_info_len: usize) -> *mut u8 { - ptr::null_mut() + core::ptr::null_mut() } +#[link(wasm_import_module = "env")] unsafe extern "C" { pub fn tari_engine(op: i32, input_ptr: *const u8, input_len: usize) -> *mut u8; - pub fn debug(input_ptr: *const u8, input_len: usize); + pub fn tari_debug(input_ptr: *const u8, input_len: usize); pub fn on_panic(msg_ptr: *const u8, msg_len: u32, line: u32, column: u32); } #[cfg(feature = "unexpected_export_function")] #[unsafe(no_mangle)] pub extern "C" fn i_shouldnt_be_here() -> *mut u8 { - ptr::null_mut() + core::ptr::null_mut() +} + +/// 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 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"))] +mod engine_call_outside_invocation { + use std::alloc::{Layout, alloc, dealloc}; + + use super::tari_engine; + + const USIZE_SIZE: usize = size_of::(); + const USIZE_ALIGN: usize = align_of::(); + + /// Hard-coded minicbor encoding of `TemplateDef::V1(TemplateDefV1 { template_name: "Buggy", + /// abi_version: 0, functions: [FunctionDef { name: "main", arguments: [], output: Type::Unit, + /// is_mut: false, is_migration: false }] })`, with the 4-byte little-endian length prefix that + /// `encode_for_wasm_embedding` adds. The one declared function makes the template callable, so + /// the engine drives the `tari_alloc` of the `CallInfo` and the `tari_free` of the returned + /// pointer. + #[unsafe(no_mangle)] + pub static _ABI_TEMPLATE_DEF: [u8; 28] = [ + 28, 0, 0, 0, 130, 0, 129, 131, 101, 66, 117, 103, 103, 121, 0, 129, 133, 100, 109, 97, 105, + 110, 128, 130, 0, 128, 244, 244, + ]; + + /// Hard-coded minicbor encoding of `EmitLogArg { message: "call", level: LogLevel::Info }`. + const EMIT_LOG_ARG: [u8; 9] = [130, 100, 99, 97, 108, 108, 130, 2, 128]; + + /// `EngineOp::EmitLog` + const OP_EMIT_LOG: i32 = 0x00; + + /// Minicbor encoding of `()`, the declared return type of `main`. + const ENCODED_UNIT: [u8; 1] = [128]; + + #[unsafe(no_mangle)] + pub extern "C" fn tari_alloc(size: usize) -> *mut u8 { + #[cfg(feature = "engine_call_in_alloc")] + call_engine(); + internal_alloc(size) + } + + /// # Safety + /// `ptr` must point at the payload of an allocation made by [`tari_alloc`]. + #[unsafe(no_mangle)] + pub unsafe extern "C" fn tari_free(ptr: *mut u8) { + #[cfg(feature = "engine_call_in_free")] + call_engine(); + + if !ptr.is_null() { + unsafe { internal_free(ptr) }; + } + } + + /// Calls the engine and discards whatever comes back, including a null pointer signalling that + /// the engine refused the call. Any response is released directly rather than through + /// `tari_free`, which would recurse without bound and say nothing about what the engine does + /// with one re-entrant call. + fn call_engine() { + let response = unsafe { tari_engine(OP_EMIT_LOG, EMIT_LOG_ARG.as_ptr(), EMIT_LOG_ARG.len()) }; + if !response.is_null() { + unsafe { internal_free(response) }; + } + } + + #[unsafe(no_mangle)] + pub unsafe extern "C" fn Buggy_main(_call_info: *mut u8, _call_info_len: usize) -> *mut u8 { + let ptr = internal_alloc(ENCODED_UNIT.len()); + unsafe { ptr.copy_from_nonoverlapping(ENCODED_UNIT.as_ptr(), ENCODED_UNIT.len()) }; + ptr + } + + fn internal_alloc(size: usize) -> *mut u8 { + let alloc_size = size + USIZE_SIZE; + unsafe { + let layout = Layout::from_size_align_unchecked(alloc_size, USIZE_ALIGN); + let ptr = alloc(layout); + ptr.cast::().write(alloc_size); + ptr.add(USIZE_SIZE) + } + } + + unsafe fn internal_free(ptr: *mut u8) { + unsafe { + let alloc_ptr = ptr.sub(USIZE_SIZE); + let alloc_size = alloc_ptr.cast::().read(); + dealloc(alloc_ptr, Layout::from_size_align_unchecked(alloc_size, USIZE_ALIGN)); + } + } } diff --git a/crates/engine/tests/test.rs b/crates/engine/tests/test.rs index 605b43d60f..811a725b50 100644 --- a/crates/engine/tests/test.rs +++ b/crates/engine/tests/test.rs @@ -23,6 +23,7 @@ use std::iter; use serde::{Deserialize, Serialize}; use tari_engine::{ + runtime::RuntimeError, template::{TemplateLoaderError, TemplateModuleLoader}, wasm::{WasmExecutionError, WasmModule}, }; @@ -33,6 +34,7 @@ use tari_engine_types::{ }; use tari_ootle_common_types::substate_type::SubstateType; use tari_ootle_transaction::{Epoch, Transaction, args}; +use tari_template_abi::EngineOp; use tari_template_builtin::{ACCOUNT_TEMPLATE_ADDRESS, NFT_FAUCET_TEMPLATE_ADDRESS, all_builtin_templates}; use tari_template_lib::{ models::NonFungible, @@ -259,6 +261,49 @@ fn test_buggy_template() { )); } +/// The engine calls a template's `tari_free` on the pointer the template function returned, which +/// runs template code outside any invocation — after the invocation's window has closed and its +/// consumption has been charged. An engine call from there is refused, and the refusal fails the +/// transaction rather than being swallowed: the template ignores the null pointer it gets back and +/// returns normally, so the engine reports the refusal itself. +#[test] +fn test_engine_call_in_tari_free() { + let reason = execute_buggy_main("engine_call_in_free"); + + assert_reject_reason(reason, RuntimeError::EngineCallOutsideInvocation { + op: EngineOp::EmitLog, + }); +} + +/// The counterpart to [`test_engine_call_in_tari_free`] on the other side of the invocation: the +/// engine calls `tari_alloc` to stage the `CallInfo` before the template function runs, when no +/// invocation has been entered and no meter has been installed yet. +#[test] +fn test_engine_call_in_tari_alloc() { + let reason = execute_buggy_main("engine_call_in_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( + "Buggy", + "tests/templates/buggy", + &[feature], + iter::empty::<(String, String)>(), + ); + + test.execute_expect_failure( + Transaction::builder_localnet(Epoch(1)) + .call_function(template_addr, "main", args![]) + .build_and_seal(test.secret_key()), + vec![], + ) +} + #[test] fn test_private_function() { // instantiate the counter