Skip to content

Commit

Permalink
Avoid iloop externalizing diagnostics for invalid references (#1028)
Browse files Browse the repository at this point in the history
* Avoid iloop externalizing diagnostics for invalid references

* Return the payload in "unknown object reference" error
  • Loading branch information
brson authored Sep 5, 2023
1 parent 3d6c35d commit 243a362
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
11 changes: 9 additions & 2 deletions soroban-env-host/src/host_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use soroban_env_common::{
xdr::{ContractCostType, ScErrorCode, ScErrorType},
Compare, DurationSmall, I128Small, I256Small, I64Small, SymbolSmall, SymbolStr, Tag,
TimepointSmall, U128Small, U256Small, U64Small,
TimepointSmall, TryFromVal, U128Small, U256Small, U64Small,
};

use crate::{
Expand Down Expand Up @@ -369,11 +369,18 @@ impl Host {
} else if let Some(obj) = r.get(handle_to_index(handle)) {
f(obj)
} else {
// Discard the broken object here instead of including
// it in the error to avoid further attempts to interpret it.
// e.g. if diagnostics are on, then this would immediately
// begin recursing, attempting and failing to externalize
// debug info for this very error. Store the u64 payload instead.
let obj_payload = obj.as_val().get_payload();
let payload_val = Val::try_from_val(self, &obj_payload)?;
Err(self.err(
ScErrorType::Object,
ScErrorCode::MissingValue,
"unknown object reference",
&[obj.to_val()],
&[payload_val],
))
}
}
Expand Down
36 changes: 34 additions & 2 deletions soroban-env-host/src/test/hostile.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
use soroban_env_common::{
xdr::{ContractCostType, ScErrorCode, ScErrorType},
Env, EnvBase, Symbol, Val, VecObject,
Env, EnvBase, Symbol, Tag, Val, VecObject,
};
use soroban_synth_wasm::{Arity, ModEmitter, Operand};
use soroban_test_wasms::HOSTILE;

use crate::{budget::AsBudget, host_object::HostVec, Host, HostError};
use crate::{
budget::{AsBudget, Budget},
host_object::HostVec,
storage::Storage,
DiagnosticLevel, Host, HostError,
};

#[test]
fn hostile_iloop_traps() -> Result<(), HostError> {
Expand Down Expand Up @@ -257,3 +262,30 @@ fn excessive_memory_growth() -> Result<(), HostError> {

Ok(())
}

// Regression test for infinte loop / recursion
// while externalizing diagnostics for objects
// with invalid references.
#[test]
fn broken_object() {
fn val_from_body_and_tag(body: u64, tag: Tag) -> Val {
unsafe {
// Safety: Val is a repr(transparent) u64
const TAG_BITS: usize = 8;
std::mem::transmute((body << TAG_BITS) | (tag as u64))
}
}

let budget = Budget::default();
let storage = Storage::default();
let host = Host::with_storage_and_budget(storage, budget);

// Diagnostics must be on
host.set_diagnostic_level(DiagnosticLevel::Debug).unwrap();

// Bogus u256 object
let bad_val = val_from_body_and_tag(u64::MAX, Tag::U256Object);

// This iloops externalizing diagnostics for the error it is generating.
let _args = host.vec_new_from_slice(&[bad_val]);
}

0 comments on commit 243a362

Please sign in to comment.