Split system object storage into a fully separate vector.

soroban-env-common/src/val.rs

@@ -376,6 +376,9 @@ impl_tryfroms_and_tryfromvals_delegating_to_rawvalconvertible!(Error);
 pub trait WasmiMarshal: Sized {
     fn try_marshal_from_value(v: wasmi::Value) -> Option<Self>;
     fn marshal_from_self(self) -> wasmi::Value;
+    fn try_to_val(&self) -> Option<Val> {
+        None
+    }
 }
 
 #[cfg(feature = "wasmi")]
@@ -391,6 +394,10 @@ impl WasmiMarshal for Val {
     fn marshal_from_self(self) -> wasmi::Value {
         wasmi::Value::I64(self.get_payload() as i64)
     }
+
+    fn try_to_val(&self) -> Option<Val> {
+        Some(*self)
+    }
 }
 
 #[cfg(feature = "wasmi")]

soroban-env-common/src/wrapper_macros.rs

@@ -99,6 +99,10 @@ macro_rules! impl_wrapper_wasmi_conversions {
             fn marshal_from_self(self) -> wasmi::Value {
                 wasmi::Value::I64(self.as_val().get_payload() as i64)
             }
+
+            fn try_to_val(&self) -> Option<Val> {
+                Some(*self.as_val())
+            }
         }
     };
 }

soroban-env-host/src/host.rs

@@ -44,7 +44,6 @@ pub(crate) mod metered_vector;
 pub(crate) mod metered_xdr;
 mod num;
 mod prng;
-use bit_set::BitSet;
 pub use prng::{Seed, SEED_BYTES};
 mod validity;
 pub use error::HostError;
@@ -96,7 +95,7 @@ pub(crate) struct HostImpl {
     ledger: RefCell<Option<LedgerInfo>>,
     pub(crate) objects: RefCell<Vec<HostObject>>,
     system_mode: RefCell<bool>,
-    system_objects: RefCell<BitSet>,
+    system_objects: RefCell<Vec<HostObject>>,
     storage: RefCell<Storage>,
     pub(crate) context: RefCell<Vec<Context>>,
     // Note: budget is refcounted and is _not_ deep-cloned when you call HostImpl::deep_clone,
@@ -175,7 +174,7 @@ impl_checked_borrow_helpers!(
 );
 impl_checked_borrow_helpers!(
     system_objects,
-    BitSet,
+    Vec<HostObject>,
     try_borrow_system_objects,
     try_borrow_system_objects_mut
 );

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

@@ -6,7 +6,7 @@ use soroban_env_common::{
 use crate::{
     auth::AuthorizationManagerSnapshot,
     budget::AsBudget,
-    err,
+    err, host_object,
     storage::{InstanceStorageMap, StorageMap},
     xdr::{ContractCostType, ContractExecutable, Hash, HostFunction, HostFunctionType, ScVal},
     Error, Host, HostError, Object, Symbol, SymbolStr, TryFromVal, TryIntoVal, Val,
@@ -124,11 +124,11 @@ impl Frame {
             Frame::TestContract(tc) => (&tc.func, &tc.args),
         };
         if let Ok(obj) = Object::try_from(sym.to_val()) {
-            bs.insert(obj.get_handle() as usize);
+            bs.insert(host_object::handle_to_index(obj.get_handle()));
         }
         for v in args.iter() {
             if let Ok(obj) = Object::try_from(*v) {
-                bs.insert(obj.get_handle() as usize);
+                bs.insert(host_object::handle_to_index(obj.get_handle()));
             }
         }
 

soroban-env-host/src/host_object.rs

@@ -1,6 +1,7 @@
 use soroban_env_common::{
-    xdr::ContractCostType, Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall,
-    SymbolStr, Tag, TimepointSmall, U128Small, U256Small, U64Small,
+    xdr::{ContractCostType, ScErrorCode, ScErrorType},
+    Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall, SymbolStr, Tag,
+    TimepointSmall, U128Small, U256Small, U64Small,
 };
 
 use crate::{
@@ -190,16 +191,82 @@ declare_mem_host_object_type!(xdr::ScString, StringObject, String);
 declare_mem_host_object_type!(xdr::ScSymbol, SymbolObject, Symbol);
 declare_host_object_type!(xdr::ScAddress, AddressObject, Address);
 
+// We store system objects in a separate Vec from user objects, and
+// differentiate handles to them by the low bit of the handle: 1 means system
+// objects, 0 means user objects. This way user code is literally unaffected by
+// system-object allocation -- won't even change the handles users get -- and we
+// can also check that system objects are never leaking out of the host.
+pub fn is_system_object_handle(handle: u32) -> bool {
+    handle & 1 == 1
+}
+
+pub fn is_system_object(obj: Object) -> bool {
+    is_system_object_handle(obj.get_handle())
+}
+
+pub fn is_system_object_value(val: Val) -> bool {
+    if let Ok(obj) = Object::try_from(val) {
+        is_system_object(obj)
+    } else {
+        false
+    }
+}
+
+pub fn handle_to_index(handle: u32) -> usize {
+    (handle as usize) >> 1
+}
+
+pub fn index_to_handle(host: &Host, index: usize, system_mode: bool) -> Result<u32, HostError> {
+    if let Ok(smaller) = u32::try_from(index) {
+        if let Some(shifted) = smaller.checked_shl(1) {
+            if system_mode {
+                return Ok(shifted | 1);
+            } else {
+                return Ok(shifted | 0);
+            }
+        }
+    }
+    Err(host.err_arith_overflow())
+}
+
 impl Host {
+    pub fn check_value_is_non_system_object(&self, val: Val) -> Result<(), HostError> {
+        if is_system_object_value(val) {
+            Err(self.err(
+                ScErrorType::Context,
+                ScErrorCode::InternalError,
+                "unexpected system value",
+                &[val],
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
     // Execute `f` in system mode, restoring the state of the flag on exit (and allowing re-entry of system-mode).
     pub(crate) fn with_system_mode<T>(
         &self,
         f: impl FnOnce() -> Result<T, HostError>,
+    ) -> Result<T, HostError> {
+        self.with_system_mode_flag(true, f)
+    }
+
+    pub(crate) fn with_user_mode<T>(
+        &self,
+        f: impl FnOnce() -> Result<T, HostError>,
+    ) -> Result<T, HostError> {
+        self.with_system_mode_flag(false, f)
+    }
+
+    pub(crate) fn with_system_mode_flag<T>(
+        &self,
+        new_flag: bool,
+        f: impl FnOnce() -> Result<T, HostError>,
     ) -> Result<T, HostError> {
         let saved_flag = {
             let mut flag = self.try_borrow_system_mode_mut()?;
             let saved_flag = *flag;
-            *flag = true;
+            *flag = new_flag;
             saved_flag
         };
         let res = f();
@@ -212,43 +279,52 @@ impl Host {
         Ok(*self.try_borrow_system_mode()?)
     }
 
-    fn system_obj_allowed(&self, obj: Object) -> Result<bool, HostError> {
-        if !self.in_system_mode()? {
-            return Ok(false);
-        }
-        let objs = self.try_borrow_system_objects()?;
-        Ok(objs.contains(obj.get_handle() as usize))
-    }
-
-    fn allow_system_obj(&self, obj: Object) -> Result<(), HostError> {
-        let mut objs = self.try_borrow_system_objects_mut()?;
-        objs.insert(obj.get_handle() as usize);
-        Ok(())
+    pub(crate) fn system_to_user(&self, val: Val) -> Result<Val, HostError> {
+        let scval = self.with_system_mode(|| self.from_host_val(val))?;
+        self.with_user_mode(|| self.to_host_val(&scval))
     }
 
     fn obj_allowed(&self, obj: Object) -> Result<bool, HostError> {
-        if self.system_obj_allowed(obj)? {
+        if self.in_system_mode()? {
+            // System mode can read all objects.
             return Ok(true);
         }
         self.with_current_context_opt(|ctx| {
             Ok(if let Some(ctx) = ctx {
-                ctx.acl.contains(obj.get_handle() as usize)
+                // If we're in a specific context, there is an ACL
+                // restricting which objects can be seen.
+                if is_system_object(obj) {
+                    // ACLs never allow access to system objects.
+                    false
+                } else {
+                    ctx.acl.contains(handle_to_index(obj.get_handle()))
+                }
             } else {
                 // When there's no context, it means we're accessing the
                 // host from outside of a contract or anything (eg. in the
-                // embedding program or a test) so all objects are allowed.
+                // embedding program, or a test) so all objects are allowed.
                 true
             })
         })
     }
 
     fn allow_obj(&self, obj: Object) -> Result<(), HostError> {
-        if self.in_system_mode()? {
-            return self.allow_system_obj(obj);
+        if is_system_object(obj) {
+            if !self.in_system_mode()? {
+                return Err(self.err(
+                    ScErrorType::Context,
+                    ScErrorCode::InternalError,
+                    "allow_obj on system object in non-system mode",
+                    &[obj.to_val()],
+                ));
+            }
+            // Allowing a system object in system mode is a no-op,
+            // they're always allowed.
+            return Ok(());
         }
         self.with_current_context_mut_opt(|ctx| {
             if let Some(ctx) = ctx {
-                ctx.acl.insert(obj.get_handle() as usize);
+                ctx.acl.insert(handle_to_index(obj.get_handle()));
             }
             Ok(())
         })
@@ -269,15 +345,18 @@ impl Host {
         &self,
         hot: HOT,
     ) -> Result<HOT::Wrapper, HostError> {
-        let prev_len = self.try_borrow_objects()?.len();
-        if prev_len > u32::MAX as usize {
-            return Err(self.err_arith_overflow());
-        }
+        let system_mode = self.in_system_mode()?;
+        let mut objects = if system_mode {
+            self.try_borrow_system_objects_mut()?
+        } else {
+            self.try_borrow_objects_mut()?
+        };
+        let index = objects.len();
         // charge for the new host object, which is just the amortized cost of a single
         // `HostObject` allocation
         metered_clone::charge_heap_alloc::<HostObject>(1, self.as_budget())?;
-        self.try_borrow_objects_mut()?.push(HOT::inject(hot));
-        let handle = prev_len as u32;
+        objects.push(HOT::inject(hot));
+        let handle = index_to_handle(self, index, system_mode)?;
         let wrapper = HOT::new_from_handle(handle);
         // Any object we _create_ we're implicitly allowed to access.
         self.allow_obj(wrapper.clone().into())?;
@@ -295,22 +374,18 @@ impl Host {
         F: FnOnce(Option<&HostObject>) -> Result<U, HostError>,
     {
         self.charge_budget(ContractCostType::VisitObject, None)?;
-        let r = self.try_borrow_objects()?;
         let obj: Object = obj.into();
+        let handle: u32 = obj.get_handle();
+        let index = handle_to_index(handle);
+        let objects = if is_system_object_handle(handle) {
+            self.try_borrow_system_objects()?
+        } else {
+            self.try_borrow_objects()?
+        };
         if self.obj_allowed(obj)? {
-            let handle: u32 = obj.get_handle();
-            f(r.get(handle as usize))
+            f(objects.get(index))
         } else {
-            eprintln!(
-                "access denied to object {:?} = {:?}",
-                obj,
-                r.get(obj.get_handle() as usize)
-            );
-            eprintln!(
-                "context ACL: {:?}",
-                self.with_current_context(|ctx| Ok(ctx.acl.clone()))?
-            );
-            eprintln!("system ACL: {:?}", self.try_borrow_system_objects()?);
+            self.log_diagnostics("denied access to object", &[obj.to_val()])?;
             f(None)
         }
     }

soroban-env-host/src/native_contract/account_contract.rs

@@ -126,25 +126,34 @@ pub(crate) fn check_account_contract_auth(
     signature_args: &Vec<Val>,
     invocation: &AuthorizedInvocation,
 ) -> Result<(), HostError> {
-    let payload_obj = host.bytes_new_from_slice(signature_payload)?;
-    let signature_args_vec = HostVec::try_from_val(host, signature_args)?;
+    let signature_args_vec = HostVec::try_from_val(host, &signature_args)?;
     let mut auth_context_vec = HostVec::new(host)?;
     invocation_tree_to_auth_contexts(host, invocation, &mut auth_context_vec)?;
-    Ok(host
-        .call_n_internal(
-            account_contract,
-            ACCOUNT_CONTRACT_CHECK_AUTH_FN_NAME.try_into_val(host)?,
-            &[
-                payload_obj.into(),
-                signature_args_vec.into(),
-                auth_context_vec.into(),
-            ],
-            // Allow self reentry for this function in order to be able to do
-            // wallet admin ops using the auth framework itself.
-            ContractReentryMode::SelfAllowed,
-            true,
-        )?
-        .try_into()?)
+
+    // Copy structured object types to user objects, to allow access by user code.
+    let signature_args_vec =
+        HostVec::try_from_val(host, &host.system_to_user(signature_args_vec.into())?)?;
+    let auth_context_vec =
+        HostVec::try_from_val(host, &host.system_to_user(auth_context_vec.into())?)?;
+
+    host.with_user_mode(|| {
+        let payload_obj = host.bytes_new_from_slice(signature_payload)?;
+        Ok(host
+            .call_n_internal(
+                account_contract,
+                ACCOUNT_CONTRACT_CHECK_AUTH_FN_NAME.try_into_val(host)?,
+                &[
+                    payload_obj.into(),
+                    signature_args_vec.into(),
+                    auth_context_vec.into(),
+                ],
+                // Allow self reentry for this function in order to be able to do
+                // wallet admin ops using the auth framework itself.
+                ContractReentryMode::SelfAllowed,
+                true,
+            )?
+            .try_into()?)
+    })
 }
 
 pub(crate) fn check_account_authentication(

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

@@ -17,7 +17,7 @@ fn vec_as_seen_by_host() -> Result<(), HostError> {
     let obj0: Object = val0.try_into()?;
     let obj1: Object = val1.try_into()?;
     assert_eq!(obj0.get_handle(), 0);
-    assert_eq!(obj1.get_handle(), 1);
+    assert_eq!(obj1.get_handle(), 2);
     assert_eq!(obj0.as_val().get_tag(), Tag::VecObject);
     assert_eq!(obj1.as_val().get_tag(), Tag::VecObject);
     // Check that we got 2 distinct Vec objects

soroban-env-host/src/vm/dispatch.rs

@@ -94,8 +94,11 @@ macro_rules! generate_dispatch_functions {
 
                     let res = match res {
                         Ok(ok) => {
-                            let val: Value = ok.marshal_from_self();
-                            if let Value::I64(v) = val {
+                            if let Some(val) = ok.try_to_val() {
+                                host.check_value_is_non_system_object(val)?;
+                            }
+                            let value: Value = ok.marshal_from_self();
+                            if let Value::I64(v) = value {
                                 Ok((v,))
                             } else {
                                 Err(BadSignature.into())