fix(engine)!: reject engine calls made outside a template invocation

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}")]

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<u32, WasmPtr<u8>>;
 pub(crate) type WasmFreeFn = TypedFunction<WasmPtr<u8>, ()>;
 
-#[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<T> {
     memory: Option<Memory>,
     state: T,
     mem_alloc: Option<WasmAllocFn>,
     mem_free: Option<WasmFreeFn>,
-    last_panic: Arc<Mutex<Option<String>>>,
-    last_engine_error: Arc<Mutex<Option<RuntimeError>>>,
-    invocation_meter: Arc<Mutex<Option<InvocationMeter>>>,
+    last_panic: Option<String>,
+    last_engine_error: Option<RuntimeError>,
+    invocation_meter: Option<InvocationMeter>,
+    in_template_invocation: bool,
+    refused_engine_call: Option<EngineOp>,
 }
 
 /// Per-invocation view of the Wasmer meter, letting host calls read the in-flight consumption of
@@ -74,17 +75,56 @@ impl<T> WasmEnv<T> {
             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<EngineOp> {
+        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<T> WasmEnv<T> {
 
     /// 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<S: AsStoreMut>(&self, store: &mut S) -> Option<u64> {
+    pub(super) fn take_unsynced_in_flight_points<S: AsStoreMut>(&mut self, store: &mut S) -> Option<u64> {
         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<T> WasmEnv<T> {
         Some(delta)
     }
 
-    fn invocation_meter_mut(&self) -> MutexGuard<'_, Option<InvocationMeter>> {
-        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<S: AsStoreMut>(&self, store: &mut S, len: u32) -> Result<WasmPtr<u8>, 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<S: AsStoreMut>(&self, store: &mut S, ptr: WasmPtr<u8>) -> 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<String>> {
-        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<WasmAllocFn, WasmExecutionError> {
+        self.mem_alloc
+            .clone()
+            .ok_or(WasmExecutionError::MissingAbiFunction { function: "tari_alloc" })
     }
 
-    pub(super) fn take_last_panic_message(&self) -> Option<String> {
-        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<WasmFreeFn, WasmExecutionError> {
+        self.mem_free
+            .clone()
+            .ok_or(WasmExecutionError::MissingAbiFunction { function: "tari_free" })
     }
 
-    fn last_engine_error_mut(&self) -> MutexGuard<'_, Option<RuntimeError>> {
-        self.last_engine_error.lock().expect("last_engine_error poisoned")
+    pub(super) fn take_last_panic_message(&mut self) -> Option<String> {
+        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<RuntimeError> {
-        self.last_engine_error_mut().take()
+    pub(super) fn take_last_engine_error(&mut self) -> Option<RuntimeError> {
+        self.last_engine_error.take()
     }
 
     pub(super) fn load_template_def<S: AsStoreMut>(
@@ -292,12 +327,6 @@ impl<T> WasmEnv<T> {
         &mut self.state
     }
 
-    fn get_mem_alloc_func(&self) -> Result<&TypedFunction<u32, WasmPtr<u8>>, 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)