Repository: stellar/rs-soroban-env (rust)

Issue:
do pre_release number check on upload / RPC
We should reject contracts with non-matching pre-release numbers earlier than just invocation -- also on upload or submission-to-the-RPC

Pull request:
Tighten wasm interface version checks, and do on upload. Fix #1052.
This fixes #1052 by rejecting contracts with unsupported versions during upload (they are already rejected during instantiation/execution). It also slightly tightens the set of allowed contracts, rejecting old-protocol ones with nonzero prerelease and rejecting all future protocol versions.


Patch:
diff --git a/soroban-env-common/src/meta.rs b/soroban-env-common/src/meta.rs
index 80413f2f..f5add576 100644
--- a/soroban-env-common/src/meta.rs
+++ b/soroban-env-common/src/meta.rs
@@ -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
 }
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index 4bde5f1a..f6a9dafa 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -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.
@@ -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 {
@@ -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,
@@ -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> {
diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs
index 9679dd46..cbb95baf 100644
--- a/soroban-env-host/src/vm.rs
+++ b/soroban-env-host/src/vm.rs
@@ -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.
@@ -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,


Test patch:
diff --git a/soroban-env-host/src/test/complex.rs b/soroban-env-host/src/test/complex.rs
index c9f3279a..163ef7eb 100644
--- a/soroban-env-host/src/test/complex.rs
+++ b/soroban-env-host/src/test/complex.rs
@@ -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],
diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs
index d28af48f..44f1b9fe 100644
--- a/soroban-env-host/src/test/hostile.rs
+++ b/soroban-env-host/src/test/hostile.rs
@@ -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
diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs
index 448ec221..b0d09d43 100644
--- a/soroban-env-host/src/test/invocation.rs
+++ b/soroban-env-host/src/test/invocation.rs
@@ -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(())
 }
 
diff --git a/soroban-env-host/src/test/ledger.rs b/soroban-env-host/src/test/ledger.rs
index e98b02bd..9b961c27 100644
--- a/soroban-env-host/src/test/ledger.rs
+++ b/soroban-env-host/src/test/ledger.rs
@@ -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],
diff --git a/soroban-env-host/src/test/lifecycle.rs b/soroban-env-host/src/test/lifecycle.rs
index 8c3e2239..0281e606 100644
--- a/soroban-env-host/src/test/lifecycle.rs
+++ b/soroban-env-host/src/test/lifecycle.rs
@@ -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()
     })
@@ -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());
@@ -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 {
diff --git a/soroban-env-host/src/test/metering_benchmark.rs b/soroban-env-host/src/test/metering_benchmark.rs
index d09340bc..9ebe29f2 100644
--- a/soroban-env-host/src/test/metering_benchmark.rs
+++ b/soroban-env-host/src/test/metering_benchmark.rs
@@ -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],
diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs
index 7a51ea6b..50a76022 100644
--- a/soroban-env-host/src/test/token.rs
+++ b/soroban-env-host/src/test/token.rs
@@ -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],
diff --git a/soroban-env-host/src/test/util.rs b/soroban-env-host/src/test/util.rs
index a517775a..af7f409b 100644
--- a/soroban-env-host/src/test/util.rs
+++ b/soroban-env-host/src/test/util.rs
@@ -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],