Skip to content

Commit

Permalink
Tighten wasm interface version checks, and do on upload. Fix #1052. (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
graydon authored Sep 11, 2023
1 parent 66a3c50 commit 494ce9f
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 53 deletions.
4 changes: 2 additions & 2 deletions soroban-env-common/src/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ soroban_env_macros::generate_env_meta_consts!(
pre_release_version: 57,
);

pub fn get_ledger_protocol_version(interface_version: u64) -> u32 {
pub const fn get_ledger_protocol_version(interface_version: u64) -> u32 {
// The ledger protocol version is the high 32 bits of INTERFACE_VERSION
(interface_version >> 32) as u32
}

pub fn get_pre_release_version(interface_version: u64) -> u32 {
pub const fn get_pre_release_version(interface_version: u64) -> u32 {
// The pre-release version is the low 32 bits of INTERFACE_VERSION
interface_version as u32
}
46 changes: 37 additions & 9 deletions soroban-env-host/src/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ impl Host {
}
}

pub fn get_ledger_protocol_version(&self) -> Result<u32, HostError> {
self.with_ledger_info(|li| Ok(li.protocol_version))
}

/// Helper for mutating the [`Budget`] held in this [`Host`], either to
/// allocate it on contract creation or to deplete it on callbacks from
/// the VM or host functions.
Expand Down Expand Up @@ -756,6 +760,37 @@ impl Host {
&[],
)
})?;

// Check size before instantiation.
let wasm_bytes_m: crate::xdr::BytesM = wasm.try_into().map_err(|_| {
self.err(
ScErrorType::Value,
ScErrorCode::ExceededLimit,
"Wasm code is too large",
&[],
)
})?;

// Instantiate a temporary / throwaway VM using this wasm. This will do
// both quick checks like "does this wasm have the right protocol number
// to run on this network" and also a full parse-and-link pass to check
// that the wasm is basically not garbage. It might still fail to run
// but it will at least instantiate. This might seem a bit heavyweight
// but really "instantiating a VM" is mostly just "parsing the module
// and doing those checks" anyway. Revisit in the future if you want to
// try to split these costs up some.
if cfg!(any(test, feature = "testutils")) && wasm_bytes_m.as_slice().is_empty() {
// Allow a zero-byte contract when testing, as this is used to make
// native test contracts behave like wasm. They will never be
// instantiated, this is just to exercise their storage logic.
} else {
let _check_vm = Vm::new(
self,
Hash(hash_bytes.metered_clone(self)?),
wasm_bytes_m.as_slice(),
)?;
}

let hash_obj = self.add_host_object(self.scbytes_from_slice(hash_bytes.as_slice())?)?;
let code_key = Rc::metered_new(
LedgerKey::ContractCode(LedgerKeyContractCode {
Expand All @@ -772,14 +807,7 @@ impl Host {
let data = LedgerEntryData::ContractCode(ContractCodeEntry {
hash: Hash(hash_bytes),
ext: ExtensionPoint::V0,
code: wasm.try_into().map_err(|_| {
self.err(
ScErrorType::Value,
ScErrorCode::ExceededLimit,
"Wasm code is too large",
&[],
)
})?,
code: wasm_bytes_m,
});
storage.put(
&code_key,
Expand Down Expand Up @@ -1344,7 +1372,7 @@ impl VmCallerEnv for Host {
}

fn get_ledger_version(&self, _vmcaller: &mut VmCaller<Host>) -> Result<U32Val, Self::Error> {
self.with_ledger_info(|li| Ok(li.protocol_version.into()))
Ok(self.get_ledger_protocol_version()?.into())
}

fn get_ledger_sequence(&self, _vmcaller: &mut VmCaller<Host>) -> Result<U32Val, Self::Error> {
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use super::util::{generate_account_id, generate_bytes_array};
#[test]
fn run_complex() -> Result<(), HostError> {
let info = crate::LedgerInfo {
protocol_version: 21,
protocol_version: crate::meta::get_ledger_protocol_version(crate::meta::INTERFACE_VERSION),
sequence_number: 1234,
timestamp: 1234,
network_id: [7; 32],
Expand Down
5 changes: 3 additions & 2 deletions soroban-env-host/src/test/hostile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,11 @@ fn wasm_module_with_mem_grow(n_pages: usize) -> Vec<u8> {
fn excessive_memory_growth() -> Result<(), HostError> {
// not sure why calling `memory_grow(32)`, wasmi requests 33 pages of memory
let wasm = wasm_module_with_mem_grow(32);
let host = Host::test_host_with_recording_footprint()
let host = Host::test_host_with_recording_footprint();
let contract_id_obj = host.register_test_contract_wasm(wasm.as_slice());
let host = host
.test_budget(0, 0)
.enable_model(ContractCostType::WasmMemAlloc, 0, 0, 1, 0);
let contract_id_obj = host.register_test_contract_wasm(wasm.as_slice());
host.set_diagnostic_level(crate::DiagnosticLevel::Debug)?;

// This one should just run out of memory
Expand Down
7 changes: 4 additions & 3 deletions soroban-env-host/src/test/invocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ fn invoke_alloc() -> Result<(), HostError> {
// So we wind up with a growth-sequence that's a bit irregular: +0x10000,
// +0x20000, +0x30000, +0x50000, +0x90000. Total is 1 + 2 + 3 + 5 + 9 = 20
// pages or about 1.3 MiB, plus the initial 17 pages (1.1MiB) plus some more
// slop from general host machinery allocations, we get around 2.5MiB. Call
// is "less than 3MiB".
// slop from general host machinery allocations, plus allocating a VM once
// during upload and once during execution we get around 2.5MiB. Call
// is "less than 4MiB".
assert!(used_bytes > (128 * 4096));
assert!(used_bytes < 0x30_0000);
assert!(used_bytes < 0x40_0000);
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ fn ledger_network_id() -> Result<(), HostError> {

let host = Host::with_storage_and_budget(storage, budget);
host.set_ledger_info(LedgerInfo {
protocol_version: 0,
protocol_version: crate::meta::get_ledger_protocol_version(crate::meta::INTERFACE_VERSION),
sequence_number: 0,
timestamp: 0,
network_id: [7; 32],
Expand Down
6 changes: 3 additions & 3 deletions soroban-env-host/src/test/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ fn test_host() -> Host {
Storage::with_enforcing_footprint_and_map(Footprint::default(), StorageMap::new());
let host = Host::with_storage_and_budget(storage, budget);
host.set_ledger_info(LedgerInfo {
protocol_version: crate::meta::get_ledger_protocol_version(crate::meta::INTERFACE_VERSION),
network_id: generate_bytes_array(),
..Default::default()
})
Expand Down Expand Up @@ -167,7 +168,7 @@ fn create_contract_using_parent_id_test() {
});

let child_id = sha256_hash_id_preimage(child_pre_image);
let child_wasm: &[u8] = b"70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4";
let child_wasm: &[u8] = &[];

// Install the code for the child contract.
let wasm_hash = xdr::Hash(Sha256::digest(&child_wasm).try_into().unwrap());
Expand Down Expand Up @@ -235,8 +236,7 @@ fn create_contract_using_parent_id_test() {

#[test]
fn create_contract_from_source_account() {
let code: &[u8] = b"70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4";
test_create_contract_from_source_account(&test_host(), code);
test_create_contract_from_source_account(&test_host(), &[]);
}

pub(crate) fn sha256_hash_id_preimage<T: xdr::WriteXdr>(pre_image: T) -> xdr::Hash {
Expand Down
2 changes: 1 addition & 1 deletion soroban-env-host/src/test/metering_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use super::util::{generate_account_id, generate_bytes_array};
// RUST_TEST_THREADS=1 cargo test --release --package soroban-env-host --lib -- test::metering_benchmark --nocapture --ignored

const LEDGER_INFO: LedgerInfo = LedgerInfo {
protocol_version: 21,
protocol_version: crate::meta::get_ledger_protocol_version(crate::meta::INTERFACE_VERSION),
sequence_number: 1234,
timestamp: 1234,
network_id: [7; 32],
Expand Down
4 changes: 3 additions & 1 deletion soroban-env-host/src/test/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ impl TokenTest {
fn setup() -> Self {
let host = Host::test_host_with_recording_footprint();
host.set_ledger_info(LedgerInfo {
protocol_version: 20,
protocol_version: crate::meta::get_ledger_protocol_version(
crate::meta::INTERFACE_VERSION,
),
sequence_number: 123,
timestamp: 123456,
network_id: [5; 32],
Expand Down
4 changes: 3 additions & 1 deletion soroban-env-host/src/test/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ impl Host {
let storage = Storage::with_recording_footprint(snapshot_source);
let host = Host::with_storage_and_budget(storage, Budget::default());
host.set_ledger_info(LedgerInfo {
protocol_version: 20,
protocol_version: crate::meta::get_ledger_protocol_version(
crate::meta::INTERFACE_VERSION,
),
sequence_number: 0,
timestamp: 0,
network_id: [0; 32],
Expand Down
102 changes: 73 additions & 29 deletions soroban-env-host/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,71 @@ pub struct VmFunction {
}

impl Vm {
fn check_contract_interface_version(
host: &Host,
interface_version: u64,
) -> Result<(), HostError> {
let want_proto = {
let ledger_proto = host.get_ledger_protocol_version()?;
let env_proto = get_ledger_protocol_version(meta::INTERFACE_VERSION);
if ledger_proto <= env_proto {
// ledger proto should be before or equal to env proto
ledger_proto
} else {
return Err(err!(
host,
(ScErrorType::Context, ScErrorCode::InternalError),
"ledger protocol number is ahead of supported env protocol number",
ledger_proto,
env_proto
));
}
};

let got_pre = get_pre_release_version(interface_version);
let got_proto = get_ledger_protocol_version(interface_version);

if got_proto < want_proto {
// Old protocols are finalized, we only support contracts
// with similarly finalized (zero) prerelease numbers.
if got_pre != 0 {
return Err(err!(
host,
(ScErrorType::WasmVm, ScErrorCode::InvalidInput),
"contract pre-release number for old protocol is nonzero",
got_pre
));
}
} else if got_proto == want_proto {
// Current protocol might have a nonzero prerelease number; we will
// allow it only if it matches the current prerelease exactly.
let want_pre = get_pre_release_version(meta::INTERFACE_VERSION);
if want_pre != got_pre {
return Err(err!(
host,
(ScErrorType::WasmVm, ScErrorCode::InvalidInput),
"contract pre-release number for current protocol does not match host",
got_pre,
want_pre
));
}
} else {
// Future protocols we don't allow. It might be nice (in the sense
// of "allowing uploads of a future-protocol contract that will go
// live as soon as the network upgrades to it") but there's a risk
// that the "future" protocol semantics baked in to a contract
// differ from the final semantics chosen by the network, so to be
// conservative we avoid even allowing this.
return Err(err!(
host,
(ScErrorType::WasmVm, ScErrorCode::InvalidInput),
"contract protocol number is newer than host",
got_proto
));
}
Ok(())
}

fn check_meta_section(host: &Host, m: &Module) -> Result<(), HostError> {
// We check that the interface version number has the same pre-release number as
// us as well as a protocol that's less than or equal to our protocol.
Expand All @@ -85,36 +150,15 @@ impl Vm {
if let Some(env_meta_entry) = ScEnvMetaEntry::read_xdr_iter(&mut cursor).next() {
let ScEnvMetaEntry::ScEnvMetaKindInterfaceVersion(v) =
host.map_err(env_meta_entry)?;
let got_pre = get_pre_release_version(v);
let want_pre = get_pre_release_version(meta::INTERFACE_VERSION);
let got_proto = get_ledger_protocol_version(v);
let want_proto = get_ledger_protocol_version(meta::INTERFACE_VERSION);
if got_pre != want_pre {
return Err(err!(
host,
(ScErrorType::WasmVm, ScErrorCode::InvalidInput),
"contract pre-release number does not match host",
got_pre,
want_pre
));
} else if got_proto > want_proto {
return Err(err!(
host,
(ScErrorType::WasmVm, ScErrorCode::InvalidInput),
"contract ledger protocol number exceeds host",
got_proto,
want_proto
));
} else {
return Ok(());
}
Vm::check_contract_interface_version(host, v)
} else {
Err(host.err(
ScErrorType::WasmVm,
ScErrorCode::InvalidInput,
"contract missing environment interface version",
&[],
))
}
Err(host.err(
ScErrorType::WasmVm,
ScErrorCode::InvalidInput,
"contract missing environment interface version",
&[],
))
} else {
Err(host.err(
ScErrorType::WasmVm,
Expand Down

0 comments on commit 494ce9f

Please sign in to comment.