Enforce object handle integrity when inserting into containers, fix #569

 (#928)

soroban-env-host/src/host.rs

@@ -1000,6 +1000,9 @@ impl EnvBase for Host {
  for k in keys.iter() {
  key_syms.push(Symbol::try_from_val(self, k)?);
  }
+ for v in vals.iter() {
+ self.check_val_integrity(*v)?;
+ }
  let pair_iter = key_syms
  .iter()
  .map(|s| s.to_val())
@@ -1049,8 +1052,11 @@ impl EnvBase for Host {
  }
 
  fn vec_new_from_slice(&self, vals: &[Val]) -> Result<VecObject, Self::Error> {
- let map = HostVec::from_exact_iter(vals.iter().cloned(), self.budget_ref())?;
- self.add_host_object(map)
+ let vec = HostVec::from_exact_iter(vals.iter().cloned(), self.budget_ref())?;
+ for v in vec.iter() {
+ self.check_val_integrity(*v)?;
+ }
+ self.add_host_object(vec)
  }
 
  fn vec_unpack_to_slice(&self, vec: VecObject, vals: &mut [Val]) -> Result<Void, Self::Error> {
@@ -1481,6 +1487,8 @@ impl VmCallerEnv for Host {
  k: Val,
  v: Val,
  ) -> Result<MapObject, HostError> {
+ self.check_val_integrity(k)?;
+ self.check_val_integrity(v)?;
  let mnew = self.visit_obj(m, |hm: &HostMap| hm.insert(k, v, self))?;
  self.add_host_object(mnew)
  }
@@ -1659,6 +1667,9 @@ impl VmCallerEnv for Host {
  vals.as_mut_slice(),
  |buf| Val::from_payload(u64::from_le_bytes(*buf)),
  )?;
+ for v in vals.iter() {
+ self.check_val_integrity(*v)?;
+ }
 
  // Step 3: turn pairs into a map.
  let pair_iter = key_syms
@@ -1742,6 +1753,7 @@ impl VmCallerEnv for Host {
  x: Val,
  ) -> Result<VecObject, HostError> {
  let i: u32 = i.into();
+ self.check_val_integrity(x)?;
  let vnew = self.visit_obj(v, move |hv: &HostVec| {
  self.validate_index_lt_bound(i, hv.len())?;
  hv.set(i as usize, x, self.as_budget())
@@ -1786,6 +1798,7 @@ impl VmCallerEnv for Host {
  v: VecObject,
  x: Val,
  ) -> Result<VecObject, HostError> {
+ self.check_val_integrity(x)?;
  let vnew = self.visit_obj(v, move |hv: &HostVec| hv.push_front(x, self.as_budget()))?;
  self.add_host_object(vnew)
  }
@@ -1805,6 +1818,7 @@ impl VmCallerEnv for Host {
  v: VecObject,
  x: Val,
  ) -> Result<VecObject, HostError> {
+ self.check_val_integrity(x)?;
  let vnew = self.visit_obj(v, move |hv: &HostVec| hv.push_back(x, self.as_budget()))?;
  self.add_host_object(vnew)
  }
@@ -1838,6 +1852,7 @@ impl VmCallerEnv for Host {
  x: Val,
  ) -> Result<VecObject, HostError> {
  let i: u32 = i.into();
+ self.check_val_integrity(x)?;
  let vnew = self.visit_obj(v, move |hv: &HostVec| {
  self.validate_index_le_bound(i, hv.len())?;
  hv.insert(i as usize, x, self.as_budget())
@@ -1942,6 +1957,9 @@ impl VmCallerEnv for Host {
  vals.as_mut_slice(),
  |buf| Val::from_payload(u64::from_le_bytes(*buf)),
  )?;
+ for v in vals.iter() {
+ self.check_val_integrity(*v)?;
+ }
  self.add_host_object(HostVec::from_vec(vals)?)
  }
 
@@ -1974,6 +1992,8 @@ impl VmCallerEnv for Host {
  t: StorageType,
  f: Val,
  ) -> Result<Void, HostError> {
+ self.check_val_integrity(k)?;
+ self.check_val_integrity(v)?;
  match t {
  StorageType::Temporary | StorageType::Persistent => {
  self.put_contract_data_into_ledger(k, v, t, f)?

soroban-env-host/src/host_object.rs

@@ -1,6 +1,6 @@
 use soroban_env_common::{
  xdr::ContractCostType, Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall,
- SymbolStr, TimepointSmall, U128Small, U256Small, U64Small,
+ SymbolStr, Tag, TimepointSmall, U128Small, U256Small, U64Small,
 };
 
 use crate::{
@@ -206,6 +206,48 @@ impl Host {
  f(r.get(handle as usize))
  }
 
+ pub(crate) fn check_val_integrity(&self, val: Val) -> Result<(), HostError> {
+ if let Ok(obj) = Object::try_from(val) {
+ self.check_obj_integrity(obj)?;
+ }
+ Ok(())
+ }
+
+ pub(crate) fn check_obj_integrity(&self, obj: Object) -> Result<(), HostError> {
+ unsafe {
+ self.unchecked_visit_val_obj(obj, |hopt| match hopt {
+ None => Err(self.err(
+ xdr::ScErrorType::Object,
+ xdr::ScErrorCode::MissingValue,
+ "unknown object reference",
+ &[],
+ )),
+ Some(hobj) => match (hobj, obj.to_val().get_tag()) {
+ (HostObject::Vec(_), Tag::VecObject)
+ | (HostObject::Map(_), Tag::MapObject)
+ | (HostObject::U64(_), Tag::U64Object)
+ | (HostObject::I64(_), Tag::I64Object)
+ | (HostObject::TimePoint(_), Tag::TimepointObject)
+ | (HostObject::Duration(_), Tag::DurationObject)
+ | (HostObject::U128(_), Tag::U128Object)
+ | (HostObject::I128(_), Tag::I128Object)
+ | (HostObject::U256(_), Tag::U256Object)
+ | (HostObject::I256(_), Tag::I256Object)
+ | (HostObject::Bytes(_), Tag::BytesObject)
+ | (HostObject::String(_), Tag::StringObject)
+ | (HostObject::Symbol(_), Tag::SymbolObject)
+ | (HostObject::Address(_), Tag::AddressObject) => Ok(()),
+ _ => Err(self.err(
+ xdr::ScErrorType::Object,
+ xdr::ScErrorCode::UnexpectedType,
+ "mis-tagged object reference",
+ &[],
+ )),
+ },
+ })
+ }
+ }
+
  // Notes on metering: object visiting part is covered by unchecked_visit_val_obj. Closure function
  // needs to be metered separately.
  pub(crate) fn visit_obj<HOT: HostObjectType, F, U>(

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

@@ -155,9 +155,10 @@ fn map_insert_key_vec_obj() -> Result<(), HostError> {
  host.map_put(m, k1.into(), v1)?;
 
  host.with_budget(|budget| {
- // 6 = 1 visit map + 1 visit k1 + (obj_cmp which needs to) 1 visit both k0 and k1 during lookup,
- // and then 2 more to validate order of resulting map.
- assert_eq!(budget.get_tracker(ContractCostType::VisitObject)?.0, 6);
+ // 6 = 2 visits to ensure object handle integrity + 1 visit map + 1
+ // visit k1 + (obj_cmp which needs to) 1 visit both k0 and k1 during
+ // lookup, and then 2 more to validate order of resulting map.
+ assert_eq!(budget.get_tracker(ContractCostType::VisitObject)?.0, 8);
  // upper bound of number of map-accesses, counting both binary-search, point-access and validate-scan.
  assert_eq!(budget.get_tracker(ContractCostType::MapEntry)?.0, 8);
  Ok(())

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

@@ -302,3 +302,38 @@ fn map_stack_no_overflow_65536_boxed_keys_and_vals() {
  }
  }
 }
+
+#[test]
+fn map_build_bad_element_integrity() -> Result<(), HostError> {
+ use crate::EnvBase;
+ let host = Host::default();
+ let obj = host.map_new()?;
+
+ let ok_val = obj.to_val();
+ let payload = ok_val.get_payload();
+
+ // The low 8 bits of an object-handle payload are the
+ // tag indicating its type. We just add one to the
+ // object type here, corrupting it.
+ let bad_tag = Val::from_payload(payload + 1);
+
+ // the high 32 bits of an object-handle payload are the
+ // index number of the handle. We corrupt those here with
+ // an object index far greater than any allocated.
+ let bad_handle = Val::from_payload(payload | 0xff_u64 << 48);
+
+ // Inserting ok object referejces into maps should work.
+ assert!(host.map_put(obj, ok_val, ok_val).is_ok());
+ assert!(host.map_new_from_slices(&["hi"], &[ok_val]).is_ok());
+
+ // Inserting corrupt object references into maps should fail.
+ assert!(host.map_put(obj, ok_val, bad_tag).is_err());
+ assert!(host.map_put(obj, bad_tag, ok_val).is_err());
+ assert!(host.map_new_from_slices(&["hi"], &[bad_tag]).is_err());
+
+ assert!(host.map_put(obj, ok_val, bad_handle).is_err());
+ assert!(host.map_put(obj, bad_handle, ok_val).is_err());
+ assert!(host.map_new_from_slices(&["hi"], &[bad_handle]).is_err());
+
+ Ok(())
+}

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

@@ -294,3 +294,43 @@ fn vec_binary_search() -> Result<(), HostError> {
  assert_eq!(u64::from(4u32), res);
  Ok(())
 }
+
+#[test]
+fn vec_build_bad_element_integrity() -> Result<(), HostError> {
+ use crate::EnvBase;
+ let host = Host::default();
+ let obj = host.test_vec_obj::<u32>(&[1, 2, 3])?;
+ let i = U32Val::from(1);
+
+ let ok_val = obj.to_val();
+ let payload = ok_val.get_payload();
+
+ // The low 8 bits of an object-handle payload are the
+ // tag indicating its type. We just add one to the
+ // object type here, corrupting it.
+ let bad_tag = Val::from_payload(payload + 1);
+
+ // the high 32 bits of an object-handle payload are the
+ // index number of the handle. We corrupt those here with
+ // an object index far greater than any allocated.
+ let bad_handle = Val::from_payload(payload | 0xff_u64 << 48);
+
+ // Inserting ok object referejces into vectors should work.
+ assert!(host.vec_put(obj, i, ok_val).is_ok());
+ assert!(host.vec_push_front(obj, ok_val).is_ok());
+ assert!(host.vec_push_back(obj, ok_val).is_ok());
+ assert!(host.vec_new_from_slice(&[ok_val]).is_ok());
+
+ // Inserting corrupt object references into vectors should fail.
+ assert!(host.vec_put(obj, i, bad_tag).is_err());
+ assert!(host.vec_push_front(obj, bad_tag).is_err());
+ assert!(host.vec_push_back(obj, bad_tag).is_err());
+ assert!(host.vec_new_from_slice(&[bad_tag]).is_err());
+
+ assert!(host.vec_put(obj, i, bad_handle).is_err());
+ assert!(host.vec_push_front(obj, bad_handle).is_err());
+ assert!(host.vec_push_back(obj, bad_handle).is_err());
+ assert!(host.vec_new_from_slice(&[bad_handle]).is_err());
+
+ Ok(())
+}