Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests that try to build a deep host stack. #1185

Merged
merged 6 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:
version: 0.5.16
- run: cargo hack --each-feature clippy --locked --target ${{ matrix.sys.target }}
- if: matrix.sys.test
run: cargo hack --each-feature test --locked --target ${{ matrix.sys.target }}
run: cargo hack --each-feature test --profile test-opt --locked --target ${{ matrix.sys.target }}

publish-dry-run:
if: github.event_name == 'push' || startsWith(github.head_ref, 'release/')
Expand Down
6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,9 @@ rev = "e6ba45c60c16de28c7522586b80ed0150157df73"
# soroban-wasmi_core = { path = "../wasmi/crates/core/" }

[profile.release]
codegen-units = 1
lto = true

[profile.test-opt]
inherits = "test"
opt-level = 3
debug = false
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ all: build test
test:
cargo hack --each-feature test

test-opt:
cargo hack --each-feature test --profile test-opt

build:
cargo hack --each-feature clippy
cargo hack clippy --target wasm32-unknown-unknown
Expand Down
3 changes: 3 additions & 0 deletions cackle.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ allow_unsafe = true

[pkg.soroban-env-host]
allow_unsafe = true
build.allow_apis = [
"env",
]

[pkg.unicode-ident]
allow_unsafe = true
Expand Down
1 change: 1 addition & 0 deletions soroban-env-host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ version.workspace = true
readme = "../README.md"
edition = "2021"
rust-version = "1.71"
build = "build.rs"

[dependencies]
soroban-builtin-sdk-macros = { workspace = true }
Expand Down
6 changes: 6 additions & 0 deletions soroban-env-host/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn main() {
let opt_level = std::env::var("OPT_LEVEL").unwrap_or_else(|_| "0".to_string());
if opt_level != "0" {
println!("cargo:rustc-cfg=opt_build");
}
}
22 changes: 14 additions & 8 deletions soroban-env-host/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1650,16 +1650,22 @@ impl AccountAuthorizationTracker {
let authenticate_res = self
.authenticate(host)
.map_err(|err| {
// Convert any contract errors to auth errors so that it's
// Convert any recoverable errors to auth errors so that it's
// not possible to confuse them for the errors of the
// contract that has called `require_auth`.
// Also log the original error for diagnosticts.
host.err(
ScErrorType::Auth,
ScErrorCode::InvalidAction,
"failed account authentication with error",
&[self.address.into(), err.error.to_val()],
)
// While there is no 'recovery' here, non-recoverable errors
// aren't really useful for decoration.
if err.is_recoverable() {
// Also log the original error for diagnostics.
host.err(
ScErrorType::Auth,
ScErrorCode::InvalidAction,
"failed account authentication with error",
&[self.address.into(), err.error.to_val()],
)
} else {
err
}
})
.and_then(|_| self.verify_and_consume_nonce(host));
if let Some(err) = authenticate_res.err() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
builtin_contracts::{
base_types::Address,
testutils::{authorize_single_invocation, HostVec, TestSigner},
testutils::{authorize_single_invocation, ContractTypeVec, TestSigner},
},
host_vec, Host, HostError,
};
Expand Down Expand Up @@ -50,7 +50,7 @@ impl<'a> TestStellarAssetContract<'a> {
&self,
signer: &TestSigner,
function_name: &str,
args: HostVec,
args: ContractTypeVec,
) -> Result<(), HostError> {
authorize_single_invocation(
self.host,
Expand Down
12 changes: 6 additions & 6 deletions soroban-env-host/src/builtin_contracts/testutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,18 @@ use soroban_env_common::{EnvBase, TryFromVal, Val};

use crate::builtin_contracts::base_types::BytesN;

pub(crate) use crate::builtin_contracts::base_types::Vec as HostVec;
pub(crate) use crate::builtin_contracts::base_types::Vec as ContractTypeVec;

use super::account_contract::AccountEd25519Signature;
use super::base_types::Address;

#[macro_export]
macro_rules! host_vec {
($host:expr $(,)?) => {
HostVec::new($host).unwrap()
ContractTypeVec::new($host).unwrap()
};
($host:expr, $($x:expr),+ $(,)?) => {
HostVec::from_slice($host, &[$($x.try_into_val($host).unwrap()),+]).unwrap()
ContractTypeVec::from_slice($host, &[$($x.try_into_val($host).unwrap()),+]).unwrap()
};
}

Expand Down Expand Up @@ -114,7 +114,7 @@ impl<'a> TestSigner<'a> {
let signature: Val = match self {
TestSigner::AccountInvoker(_) | TestSigner::ContractInvoker(_) => Val::VOID.into(),
TestSigner::Account(account_signer) => {
let mut signatures = HostVec::new(&host).unwrap();
let mut signatures = ContractTypeVec::new(&host).unwrap();
for key in &account_signer.signers {
signatures
.push(&sign_payload_for_account(host, key, payload))
Expand Down Expand Up @@ -143,7 +143,7 @@ pub(crate) fn authorize_single_invocation_with_nonce(
signer: &TestSigner,
contract_address: &Address,
function_name: &str,
args: HostVec,
args: ContractTypeVec,
nonce: Option<(i64, u32)>,
) {
let sc_address = signer.address(host).to_sc_address().unwrap();
Expand Down Expand Up @@ -200,7 +200,7 @@ pub(crate) fn authorize_single_invocation(
signer: &TestSigner,
contract_address: &Address,
function_name: &str,
args: HostVec,
args: ContractTypeVec,
) {
let nonce = match signer {
TestSigner::AccountInvoker(_) => None,
Expand Down
2 changes: 2 additions & 0 deletions soroban-env-host/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ mod crypto;
mod depth_limit;
mod event;
mod hostile;
#[cfg(opt_build)]
mod hostile_opt;
mod invocation;
mod ledger;
mod lifecycle;
Expand Down
1 change: 1 addition & 0 deletions soroban-env-host/src/test/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::budget::AsBudget;
use crate::builtin_contracts::base_types::Address;
use crate::builtin_contracts::testutils::{
create_account, generate_signing_key, sign_payload_for_account, signing_key_to_account_id,
ContractTypeVec,
};
use crate::{host_vec, Host, LedgerInfo};
use soroban_env_common::{AddressObject, Env, Symbol, SymbolStr, TryFromVal, TryIntoVal};
Expand Down
122 changes: 122 additions & 0 deletions soroban-env-host/src/test/hostile_opt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
use crate::budget::AsBudget;
use crate::builtin_contracts::base_types::Address;
use crate::builtin_contracts::testutils::ContractTypeVec;
use crate::{host_vec, Host, HostError, DEFAULT_HOST_DEPTH_LIMIT};
use soroban_builtin_sdk_macros::contracttype;
use soroban_env_common::{Env, Symbol, TryFromVal, TryIntoVal, Val};
use soroban_test_wasms::RECURSIVE_ACCOUNT_CONTRACT;

use crate::xdr::{
InvokeContractArgs, ScErrorType, SorobanAddressCredentials, SorobanAuthorizationEntry,
SorobanAuthorizedFunction, SorobanAuthorizedInvocation, SorobanCredentials,
};

#[contracttype]
pub enum RecursiveAccountSignature {
RedirectAddress(Address),
SerializeValue,
}

fn run_deep_host_stack_test(
contract_call_depth: u32,
serialization_depth: u32,
) -> Result<Val, HostError> {
let host = Host::test_host_with_recording_footprint();

let mut contracts = vec![];
// Reset budget to unlimited. While it's likely that the malicious
// contract would run out of budget before reaching the call stack
// depth, this simply accounts for the potential future host
// cost optimizations and contract implementation optimizations.
host.as_budget().reset_unlimited().unwrap();

// Contract instance setup:
// Contract `0` doesn't perform auth and is only used for the
// root `call` function.
// Contracts `1..CONTRACT_CALL_DEPTH - 2` recursively call
// the next contract in the sequence.
// Contract `CONTRACT_CALL_DEPTH - 1` serializes nested XDR value
// of depth `SERIALIZATION_DEPTH`.
for _ in 0..contract_call_depth {
contracts.push(
Address::try_from_val(
&host,
&host.register_test_contract_wasm(RECURSIVE_ACCOUNT_CONTRACT),
)
.unwrap(),
);
}

let mut auth_entries = vec![];
for i in 1..contract_call_depth as usize {
let signature = if i < (contract_call_depth - 1) as usize {
RecursiveAccountSignature::RedirectAddress(contracts[i + 1].clone())
} else {
RecursiveAccountSignature::SerializeValue
};
let signature_val: Val = signature.try_into_val(&host).unwrap();
let credentials = SorobanAddressCredentials {
address: contracts[i].to_sc_address().unwrap(),
nonce: 0,
signature_expiration_ledger: 1000,
signature: signature_val.try_into_val(&host).unwrap(),
};
let function_name = if i == 1 { "call" } else { "__check_auth" };
let root_invocation = SorobanAuthorizedInvocation {
function: SorobanAuthorizedFunction::ContractFn(InvokeContractArgs {
contract_address: contracts[i - 1].to_sc_address().unwrap(),
function_name: function_name.try_into().unwrap(),
args: Default::default(),
}),
sub_invocations: Default::default(),
};
auth_entries.push(SorobanAuthorizationEntry {
credentials: SorobanCredentials::Address(credentials),
root_invocation,
});
}
host.call(
contracts.last().unwrap().as_object(),
Symbol::try_from_small_str("set_depth").unwrap(),
host_vec![&host, serialization_depth].into(),
)
.unwrap();

host.set_authorization_entries(auth_entries).unwrap();
// Note, that we only are interested in the end result of the test;
// the setup shouldn't fail and doesn't need to be verified.
host.call(
contracts[0].as_object(),
Symbol::try_from_small_str("call").unwrap(),
host_vec![&host, contracts[1]].into(),
)
}

#[test]
fn test_deep_stack_call_succeeds_near_limit() {
// The serialized object has depth of `serialization_depth + 1`,
// thus the maximum serializable value needs
// `serialization_depth == DEFAULT_HOST_DEPTH_LIMIT - 1`.
let res = run_deep_host_stack_test(DEFAULT_HOST_DEPTH_LIMIT, DEFAULT_HOST_DEPTH_LIMIT - 1);
assert!(res.is_ok());
}

#[test]
fn test_deep_stack_call_fails_when_contract_call_depth_exceeded() {
let res = run_deep_host_stack_test(DEFAULT_HOST_DEPTH_LIMIT + 1, 0);
assert!(res.is_err());
let err = res.err().unwrap().error;
// We shouldn't run out of budget here, so the error would just
// be decorated as auth error.
assert!(err.is_type(ScErrorType::Auth));
}

#[test]
fn test_deep_stack_call_fails_when_serialization_depth_exceeded() {
let res = run_deep_host_stack_test(2, DEFAULT_HOST_DEPTH_LIMIT);
assert!(res.is_err());
let err = res.err().unwrap().error;
// We shouldn't run out of budget here, so the error would just
// be decorated as auth error.
assert!(err.is_type(ScErrorType::Auth));
}
13 changes: 6 additions & 7 deletions soroban-env-host/src/test/lifecycle.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::auth::RecordedAuthPayload;
use crate::builtin_contracts::testutils::HostVec;
use crate::builtin_contracts::testutils::ContractTypeVec;
use crate::{
budget::{AsBudget, Budget},
host_vec,
Expand Down Expand Up @@ -331,19 +331,19 @@ fn test_contract_wasm_update() {
ScVal::Vec(Some(ScVec(
vec![
ScVal::Symbol(ScSymbol("Wasm".try_into().unwrap())),
ScVal::Bytes(ScBytes(old_wasm_hash.0.try_into().unwrap()))
ScVal::Bytes(ScBytes(old_wasm_hash.0.try_into().unwrap())),
]
.try_into()
.unwrap()
))),
ScVal::Vec(Some(ScVec(
vec![
ScVal::Symbol(ScSymbol("Wasm".try_into().unwrap())),
ScVal::Bytes(ScBytes(updated_wasm_hash.0.try_into().unwrap()))
ScVal::Bytes(ScBytes(updated_wasm_hash.0.try_into().unwrap())),
]
.try_into()
.unwrap()
)))
))),
]
.try_into()
.unwrap(),
Expand All @@ -370,7 +370,6 @@ fn test_contract_wasm_update() {
}

#[test]

fn test_create_contract_from_source_account_recording_auth() {
let host = Host::test_host_with_recording_footprint();
let source_account = generate_account_id(&host);
Expand Down Expand Up @@ -407,8 +406,8 @@ fn test_create_contract_from_source_account_recording_auth() {
nonce: None,
invocation: SorobanAuthorizedInvocation {
function: SorobanAuthorizedFunction::CreateContractHostFn(create_contract_args),
sub_invocations: VecM::default()
}
sub_invocations: VecM::default(),
},
}]
);
}
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/stellar_asset_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
account_to_address, authorize_single_invocation,
authorize_single_invocation_with_nonce, contract_id_to_address, create_account,
generate_signing_key, new_ledger_entry_from_data, signing_key_to_account_id,
AccountContractSigner, AccountSigner, HostVec, TestSigner,
AccountContractSigner, AccountSigner, ContractTypeVec, TestSigner,
},
},
host::{frame::TestContractFrame, Frame},
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/storage.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::rc::Rc;

use crate::budget::Budget;
use crate::builtin_contracts::testutils::HostVec;
use crate::builtin_contracts::testutils::ContractTypeVec;
use crate::storage::{AccessType, Footprint};
use crate::xdr::{
ContractDataDurability, LedgerKey, LedgerKeyContractData, ScAddress, ScErrorCode, ScErrorType,
Expand Down
2 changes: 2 additions & 0 deletions soroban-test-wasms/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,6 @@ mod curr {
include_bytes!("../wasm-workspace/opt/curr/test_conditional_account.wasm").as_slice();
pub const SAC_REENTRY_TEST_CONTRACT: &[u8] =
include_bytes!("../wasm-workspace/opt/curr/sac_reentry_account.wasm").as_slice();
pub const RECURSIVE_ACCOUNT_CONTRACT: &[u8] =
include_bytes!("../wasm-workspace/opt/curr/recursive_account.wasm").as_slice();
}
8 changes: 8 additions & 0 deletions soroban-test-wasms/wasm-workspace/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion soroban-test-wasms/wasm-workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ members = [
"err",
"write_upgrade_bytes",
"conditional_account",
"sac_reentry_account"
"sac_reentry_account",
"recursive_account"
]
[profile.release]
opt-level = "z"
Expand Down
Binary file not shown.
Loading