Forbid contract reentry

soroban-env-host/src/host.rs

@@ -747,10 +747,36 @@ impl Host {
     }
 
     // Notes on metering: this is covered by the called components.
-    fn call_n(&self, contract: Object, func: Symbol, args: &[RawVal]) -> Result<RawVal, HostError> {
+    fn call_n(
+        &self,
+        contract: Object,
+        func: Symbol,
+        args: &[RawVal],
+        allow_reentry: bool,
+    ) -> Result<RawVal, HostError> {
         // Get contract ID
         let id = self.hash_from_obj_input("contract", contract)?;
 
+        if !allow_reentry {
+            for f in self.0.context.borrow().iter() {
+                let exist_id = match f {
+                    #[cfg(feature = "vm")]
+                    Frame::ContractVM(vm, _) => &vm.contract_id,
+                    Frame::Token(id, _) => id,
+                    #[cfg(feature = "testutils")]
+                    Frame::TestContract(id, _) => id,
+                    Frame::HostFunction(_) => continue,
+                };
+                if id == *exist_id {
+                    return Err(self.err_status_msg(
+                        // TODO: proper error code
+                        ScHostContextErrorCode::UnknownError,
+                        "Contract re-entry is not allowed",
+                    ));
+                }
+            }
+        }
+
         // "testutils" is not covered by budget metering.
         #[cfg(feature = "testutils")]
         {
@@ -788,7 +814,8 @@ impl Host {
                             .iter()
                             .map(|scv| self.to_host_val(scv).map(|hv| hv.val))
                             .collect::<Result<Vec<RawVal>, HostError>>()?;
-                        self.call_n(object, symbol, &args[..])
+                        // doesn't matter what `reentry` flag is passed, `HostFunction` must be the first frame
+                        self.call_n(object, symbol, &args[..], false)
                     })
                 } else {
                     Err(self.err_status_msg(
@@ -1982,10 +2009,12 @@ impl VmCallerCheckedEnv for Host {
 
         let buf = self.id_preimage_from_asset(a)?;
         let id = self.create_contract_with_id_preimage(ScContractCode::Token, buf)?;
+        // reentry is irrelavent here since this token contract has just been created
         self.call_n(
             id,
             Symbol::from_str("init_asset"),
             &[self.to_host_val(&arg)?.val],
+            false,
         )?;
 
         Ok(id)
@@ -1999,11 +2028,9 @@ impl VmCallerCheckedEnv for Host {
         func: Symbol,
         args: Object,
     ) -> Result<RawVal, HostError> {
-        let args: Vec<RawVal> = self.visit_obj(args, |hv: &HostVec| {
-            self.charge_budget(CostType::CallArgsUnpack, hv.len() as u64)?;
-            Ok(hv.iter().map(|a| a.to_raw()).collect())
-        })?;
-        let res = self.call_n(contract, func, args.as_slice());
+        let args = self.call_args_from_obj(args)?;
+        // this is the recommanded path of calling a contract, with `reentry` always set `false`
+        let res = self.call_n(contract, func, args.as_slice(), false);
         if let Err(e) = &res {
             let evt = DebugEvent::new()
                 .msg("contract call invocation resulted in error {}")
@@ -2021,9 +2048,21 @@ impl VmCallerCheckedEnv for Host {
         func: Symbol,
         args: Object,
     ) -> Result<RawVal, HostError> {
-        match self.call(vmcaller, contract, func, args) {
+        let args = self.call_args_from_obj(args)?;
+        // this is the "loosened" path of calling a contract.
+        // TODO: A `reentry` flag will be passed from `try_call` into here.
+        // For now, we are passing in `false` to disable reentry.
+        let res = self.call_n(contract, func, args.as_slice(), false);
+        match res {
             Ok(rv) => Ok(rv),
-            Err(e) => Ok(e.status.into()),
+            Err(e) => {
+                let status: RawVal = e.status.into();
+                let evt = DebugEvent::new()
+                    .msg("contract call invocation resulted in error {}")
+                    .arg(status);
+                self.record_debug_event(evt)?;
+                Ok(status)
+            }
         }
     }
 

soroban-env-host/src/host/conversion.rs

@@ -7,7 +7,7 @@ use crate::xdr::{
 use crate::{
     budget::CostType,
     events::{DebugError, CONTRACT_EVENT_TOPICS_LIMIT, TOPIC_BYTES_LENGTH_LIMIT},
-    host_object::HostObject,
+    host_object::{HostObject, HostVec},
     Host, HostError, Object, RawVal, Tag,
 };
 use ed25519_dalek::{PublicKey, Signature, SIGNATURE_LENGTH};
@@ -333,4 +333,11 @@ impl Host {
             }
         }
     }
+
+    pub(crate) fn call_args_from_obj(&self, args: Object) -> Result<Vec<RawVal>, HostError> {
+        self.visit_obj(args, |hv: &HostVec| {
+            self.charge_budget(CostType::CallArgsUnpack, hv.len() as u64)?;
+            Ok(hv.iter().map(|a| a.to_raw()).collect())
+        })
+    }
 }

soroban-env-host/src/test/invocation.rs

@@ -1,4 +1,7 @@
-use soroban_env_common::{xdr::ScVmErrorCode, RawVal};
+use soroban_env_common::{
+    xdr::{ScHostContextErrorCode, ScVmErrorCode},
+    RawVal,
+};
 
 use crate::{
     budget::Budget,
@@ -244,3 +247,30 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> {
 
     Ok(())
 }
+
+#[test]
+fn invoke_contract_with_reentry() -> Result<(), HostError> {
+    let dummy_id0 = [0; 32]; // the calling contract
+    let budget = Budget::default();
+    let storage = Host::test_storage_with_contracts(
+        vec![dummy_id0.into()],
+        vec![INVOKE_CONTRACT],
+        budget.clone(),
+    );
+    let host = Host::with_storage_and_budget(storage, budget);
+    // prepare arguments
+    let id0_obj = host.test_bin_obj(&dummy_id0)?;
+    let sym = Symbol::from_str("add_with");
+    let args = host.test_vec_obj::<i32>(&[i32::MAX, 1])?;
+    let args = host.vec_push_back(args.val, id0_obj.clone().into())?; // trying to call its own `add` function
+
+    // try call -- add will trap, and add_with will trap, but we will get a status
+    let res = host.call(id0_obj.clone().to_object(), sym.into(), args.clone().into());
+    let status = host.try_call(id0_obj.clone().to_object(), sym.into(), args.clone().into())?;
+    let code = ScHostContextErrorCode::UnknownError;
+    let exp: Status = code.into();
+    assert!(HostError::result_matches_err_status(res, code));
+    assert_eq!(status.get_payload(), exp.to_raw().get_payload());
+
+    Ok(())
+}

soroban-test-wasms/wasm-workspace/invoke_contract/src/lib.rs

@@ -5,6 +5,10 @@ pub struct Contract;
 
 #[contractimpl]
 impl Contract {
+    pub fn add(a: i32, b: i32) -> i32 {
+        a + b
+    }
+
     pub fn add_with(env: Env, x: i32, y: i32, contract_id: BytesN<32>) -> i32 {
         env.invoke_contract(
             &contract_id,