Repository: stellar/rs-soroban-env
Pull request #1291 (2023-12-05 02:57:27)

Turn off `wasm_reference_types` in Wasmi
### What

Disable wasmi `reference_types`and make all wasmi feature selections explicit.
Also did a pass-by clean up on the fuel configs code. 

### Why

`bulk_memory` and `reference_types` are both post-MVP features.
We use 1 extensively for object init/copy between host and guest. 
We do not use 2 at all. We are not using `ExternRef` and we are limiting table count to 1 (via resource limiter).

### Known limitations

[TODO or N/A]



Patch:
diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs
index 836e9006..87ad7cd4 100644
--- a/soroban-env-host/src/budget.rs
+++ b/soroban-env-host/src/budget.rs
@@ -7,6 +7,7 @@ mod wasmi_helper;
 pub(crate) use limits::DepthLimiter;
 pub use limits::{DEFAULT_HOST_DEPTH_LIMIT, DEFAULT_XDR_RW_LIMITS};
 pub use model::{MeteredCostComponent, ScaledU64};
+pub(crate) use wasmi_helper::{get_wasmi_config, load_calibrated_fuel_costs};
 
 use std::{
     cell::{RefCell, RefMut},
@@ -21,7 +22,6 @@ use crate::{
 };
 
 use dimension::{BudgetDimension, IsCpu, IsShadowMode};
-use wasmi_helper::FuelConfig;
 
 #[derive(Debug, Clone, Default, PartialEq, Eq, PartialOrd, Ord)]
 pub struct CostTracker {
@@ -137,7 +137,7 @@ pub(crate) struct BudgetImpl {
     /// For the purpose of calibration and reporting; not used for budget-limiting nor does it affect consensus
     tracker: BudgetTracker,
     is_in_shadow_mode: bool,
-    fuel_config: FuelConfig,
+    fuel_costs: wasmi::FuelCosts,
     depth_limit: u32,
 }
 
@@ -154,7 +154,7 @@ impl BudgetImpl {
             mem_bytes: BudgetDimension::try_from_config(mem_cost_params)?,
             tracker: Default::default(),
             is_in_shadow_mode: false,
-            fuel_config: Default::default(),
+            fuel_costs: load_calibrated_fuel_costs(),
             depth_limit: DEFAULT_HOST_DEPTH_LIMIT,
         };
 
@@ -246,7 +246,7 @@ impl Default for BudgetImpl {
             mem_bytes: BudgetDimension::default(),
             tracker: Default::default(),
             is_in_shadow_mode: false,
-            fuel_config: Default::default(),
+            fuel_costs: load_calibrated_fuel_costs(),
             depth_limit: DEFAULT_HOST_DEPTH_LIMIT,
         };
 
@@ -824,16 +824,4 @@ impl Budget {
     pub(crate) fn get_wasmi_fuel_remaining(&self) -> Result<u64, HostError> {
         self.0.try_borrow_mut_or_err()?.get_wasmi_fuel_remaining()
     }
-
-    // generate a wasmi fuel cost schedule based on our calibration
-    pub(crate) fn wasmi_fuel_costs(&self) -> Result<wasmi::FuelCosts, HostError> {
-        let config = &self.0.try_borrow_or_err()?.fuel_config;
-        let mut costs = wasmi::FuelCosts::default();
-        costs.base = config.base;
-        costs.entity = config.entity;
-        costs.load = config.load;
-        costs.store = config.store;
-        costs.call = config.call;
-        Ok(costs)
-    }
 }
diff --git a/soroban-env-host/src/budget/util.rs b/soroban-env-host/src/budget/util.rs
index 7a10da14..7cec780b 100644
--- a/soroban-env-host/src/budget/util.rs
+++ b/soroban-env-host/src/budget/util.rs
@@ -99,7 +99,7 @@ impl Budget {
     /// of a specific fuel category. In order to get the correct, unscaled fuel
     /// count, we have to preset all the `FuelConfig` entries to 1.
     pub fn reset_fuel_config(&self) -> Result<(), HostError> {
-        self.0.try_borrow_mut_or_err()?.fuel_config.reset();
+        self.0.try_borrow_mut_or_err()?.fuel_costs = wasmi::FuelCosts::default();
         Ok(())
     }
 
diff --git a/soroban-env-host/src/budget/wasmi_helper.rs b/soroban-env-host/src/budget/wasmi_helper.rs
index 465b4055..b0c94e37 100644
--- a/soroban-env-host/src/budget/wasmi_helper.rs
+++ b/soroban-env-host/src/budget/wasmi_helper.rs
@@ -1,55 +1,7 @@
-use super::AsBudget;
-use crate::{xdr::ContractCostType, Host};
-use wasmi::{errors, ResourceLimiter};
-
-/// This is a subset of `wasmi::FuelCosts` which are configurable, because it
-/// doesn't derive all the traits we want. These fields (coarsely) define the
-/// relative costs of different wasm instruction types and are for wasmi internal
-/// fuel metering use only. Units are in "fuels".
-#[derive(Clone)]
-pub(crate) struct FuelConfig {
-    /// The base fuel costs for all instructions.
-    pub base: u64,
-    /// The fuel cost for instruction operating on Wasm entities.
-    ///
-    /// # Note
-    ///
-    /// A Wasm entitiy is one of `func`, `global`, `memory` or `table`.
-    /// Those instructions are usually a bit more costly since they need
-    /// multiplie indirect accesses through the Wasm instance and store.
-    pub entity: u64,
-    /// The fuel cost offset for `memory.load` instructions.
-    pub load: u64,
-    /// The fuel cost offset for `memory.store` instructions.
-    pub store: u64,
-    /// The fuel cost offset for `call` and `call_indirect` instructions.
-    pub call: u64,
-}
-
-// These values are calibrated and set by us.
-impl Default for FuelConfig {
-    fn default() -> Self {
-        FuelConfig {
-            base: 1,
-            entity: 3,
-            load: 2,
-            store: 1,
-            call: 67,
-        }
-    }
-}
-
-impl FuelConfig {
-    // These values are the "factory default" and used for calibration.
-    #[cfg(any(test, feature = "testutils", feature = "bench"))]
-    pub(crate) fn reset(&mut self) {
-        self.base = 1;
-        self.entity = 1;
-        self.load = 1;
-        self.store = 1;
-        self.call = 1;
-    }
-}
+use crate::{
+    budget::AsBudget, host::error::TryBorrowOrErr, xdr::ContractCostType, Host, HostError,
+};
+use wasmi::{errors, FuelConsumptionMode, FuelCosts, ResourceLimiter};
 
 pub(crate) struct WasmiLimits {
     pub table_elements: u32,
@@ -144,3 +96,41 @@ impl ResourceLimiter for Host {
         WASMI_LIMITS_CONFIG.memories
     }
 }
+
+// These values are calibrated and set by us. Calibration is done with a given
+// wasmi version, and as long as the version is pinned, these values aren't
+// expected to change much.
+pub(crate) fn load_calibrated_fuel_costs() -> FuelCosts {
+    let mut fuel_costs = FuelCosts::default();
+    fuel_costs.base = 1;
+    fuel_costs.entity = 3;
+    fuel_costs.load = 2;
+    fuel_costs.store = 1;
+    fuel_costs.call = 67;
+    fuel_costs
+}
+
+pub(crate) fn get_wasmi_config(host: &Host) -> Result<wasmi::Config, HostError> {
+    let mut config = wasmi::Config::default();
+    let fuel_costs = host.as_budget().0.try_borrow_or_err()?.fuel_costs;
+
+    // Turn off most optional wasm features, leaving on some post-MVP features
+    // commonly enabled by Rust and Clang. Make sure all unused features are
+    // explicited turned off, so that we don't get "opted in" by a future wasmi
+    // version.
+    config
+        .consume_fuel(true)
+        .wasm_bulk_memory(true)
+        .wasm_mutable_global(true)
+        .wasm_sign_extension(true)
+        .wasm_saturating_float_to_int(false)
+        .wasm_multi_value(false)
+        .wasm_reference_types(false)
+        .wasm_tail_call(false)
+        .wasm_extended_const(false)
+        .floats(false)
+        .fuel_consumption_mode(FuelConsumptionMode::Eager)
+        .set_fuel_costs(fuel_costs);
+
+    Ok(config)
+}
diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs
index f0543f8b..2fa18d17 100644
--- a/soroban-env-host/src/vm.rs
+++ b/soroban-env-host/src/vm.rs
@@ -16,7 +16,7 @@ mod func_info;
 pub(crate) use dispatch::dummy0;
 
 use crate::{
-    budget::{AsBudget, Budget},
+    budget::{get_wasmi_config, AsBudget, Budget},
     err,
     host::{
         error::TryBorrowOrErr,
@@ -33,7 +33,7 @@ use std::{cell::RefCell, io::Cursor, rc::Rc, time::Instant};
 use fuel_refillable::FuelRefillable;
 use func_info::HOST_FUNCTIONS;
 
-use wasmi::{Engine, FuelConsumptionMode, Instance, Linker, Memory, Module, Store, Value};
+use wasmi::{Engine, Instance, Linker, Memory, Module, Store, Value};
 
 use crate::VmCaller;
 use wasmi::{Caller, StoreContextMut};
@@ -208,21 +208,7 @@ impl Vm {
             Some(module_wasm_code.len() as u64),
         )?;
 
-        let mut config = wasmi::Config::default();
-        let fuel_costs = host.as_budget().wasmi_fuel_costs()?;
-
-        // Turn off most optional wasm features, leaving on some
-        // post-MVP features commonly enabled by Rust and Clang.
-        config
-            .wasm_multi_value(false)
-            .wasm_mutable_global(true)
-            .wasm_saturating_float_to_int(false)
-            .wasm_sign_extension(true)
-            .floats(false)
-            .consume_fuel(true)
-            .fuel_consumption_mode(FuelConsumptionMode::Eager)
-            .set_fuel_costs(fuel_costs);
-
+        let config = get_wasmi_config(host)?;
         let engine = Engine::new(&config);
         let module = {
             let _span0 = tracy_span!("parse module");


Test patch:
diff --git a/soroban-env-host/observations/test__hostile__test_extern_ref_not_allowed.json b/soroban-env-host/observations/test__hostile__test_extern_ref_not_allowed.json
new file mode 100644
index 00000000..4e0958ab
--- /dev/null
+++ b/soroban-env-host/observations/test__hostile__test_extern_ref_not_allowed.json
@@ -0,0 +1,8 @@
+{
+  "   0 begin": "cpu:14488, mem:0, prngs:-/9b4a753, objs:-/-, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-",
+  "   1 call bytes_new_from_slice(46)": "cpu:14535",
+  "   2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:15506, mem:126, objs:-/1@20a4edbe",
+  "   3 call upload_wasm(Bytes(obj#1))": "",
+  "   4 ret upload_wasm -> Err(Error(WasmVm, InvalidAction))": "cpu:490934, mem:132120",
+  "   5 end": "cpu:490934, mem:132120, prngs:-/9b4a753, objs:-/1@20a4edbe, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
+}
\ No newline at end of file
diff --git a/soroban-env-host/observations/test__hostile__test_large_number_of_tables.json b/soroban-env-host/observations/test__hostile__test_large_number_of_tables.json
index cb1b1f65..1be3bf44 100644
--- a/soroban-env-host/observations/test__hostile__test_large_number_of_tables.json
+++ b/soroban-env-host/observations/test__hostile__test_large_number_of_tables.json
@@ -1,8 +1,8 @@
 {
   "   0 begin": "cpu:14488, mem:0, prngs:-/9b4a753, objs:-/-, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-",
-  "   1 call bytes_new_from_slice(78)": "cpu:14535",
-  "   2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:15514, mem:158, objs:-/1@a971f693",
+  "   1 call bytes_new_from_slice(75)": "cpu:14535",
+  "   2 ret bytes_new_from_slice -> Ok(Bytes(obj#1))": "cpu:15514, mem:155, objs:-/1@26ae1984",
   "   3 call upload_wasm(Bytes(obj#1))": "",
-  "   4 ret upload_wasm -> Err(Error(WasmVm, InvalidAction))": "cpu:504054, mem:133450",
-  "   5 end": "cpu:504054, mem:133450, prngs:-/9b4a753, objs:-/1@a971f693, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
+  "   4 ret upload_wasm -> Err(Error(WasmVm, InvalidAction))": "cpu:502826, mem:133326",
+  "   5 end": "cpu:502826, mem:133326, prngs:-/9b4a753, objs:-/1@26ae1984, vm:-/-, evt:-, store:-/-, foot:-, stk:-, auth:-/-"
 }
\ No newline at end of file
diff --git a/soroban-env-host/src/test/hostile.rs b/soroban-env-host/src/test/hostile.rs
index b8a7af40..6386abc1 100644
--- a/soroban-env-host/src/test/hostile.rs
+++ b/soroban-env-host/src/test/hostile.rs
@@ -1150,14 +1150,30 @@ fn test_large_globals() -> Result<(), HostError> {
     Ok(())
 }
 
+#[test]
+fn test_extern_ref_not_allowed() -> Result<(), HostError> {
+    let host = observe_host!(Host::test_host_with_recording_footprint());
+    host.enable_debug()?;
+    // `ExternRef` is not allowed by disabling `wasmi_reference_type`
+    let wasm = wasm_util::wasm_module_with_extern_ref();
+    let res = host.register_test_contract_wasm_from_source_account(
+        wasm.as_slice(),
+        generate_account_id(&host),
+        generate_bytes_array(&host),
+    );
+    assert!(HostError::result_matches_err(
+        res,
+        (ScErrorType::WasmVm, ScErrorCode::InvalidAction)
+    ));
+    Ok(())
+}
+
 #[test]
 fn test_large_number_of_tables() -> Result<(), HostError> {
     let host = observe_host!(Host::test_host_with_recording_footprint());
     host.enable_debug()?;
-    // even though we have enabled wasmi_reference_type, which makes multiple
-    // tables possible, we have explicitly set our table count limit to 1, in
-    // `WASMI_LIMITS_CONFIG`. Thus we essentially not allow multiple tables.
-    let wasm = wasm_util::wasm_module_with_many_tables(2);
+    // multiple tables are not allowed by disabling `wasmi_reference_type`
+    let wasm = wasm_util::wasm_module_with_additional_tables(1);
     let res = host.register_test_contract_wasm_from_source_account(
         wasm.as_slice(),
         generate_account_id(&host),
diff --git a/soroban-env-host/src/testutils.rs b/soroban-env-host/src/testutils.rs
index a763c5ed..047912b9 100644
--- a/soroban-env-host/src/testutils.rs
+++ b/soroban-env-host/src/testutils.rs
@@ -1016,14 +1016,22 @@ pub(crate) mod wasm {
         me.finish_no_validate()
     }
 
-    pub(crate) fn wasm_module_with_many_tables(n: u32) -> Vec<u8> {
+    pub(crate) fn wasm_module_with_extern_ref() -> Vec<u8> {
+        let mut me = ModEmitter::new();
+        me.table(RefType::EXTERNREF, 2, None);
+        me.custom_section(
+            soroban_env_common::meta::ENV_META_V0_SECTION_NAME,
+            &soroban_env_common::meta::XDR,
+        );
+        me.finish_no_validate()
+    }
+
+    pub(crate) fn wasm_module_with_additional_tables(n: u32) -> Vec<u8> {
         let mut me = ModEmitter::default();
-        for i in 0..n {
-            let rt = match i % 2 == 0 {
-                true => RefType::FUNCREF,
-                false => RefType::EXTERNREF,
-            };
-            me.table(rt, 2, None);
+        // by default, module already includes a table, here we are creating
+        // additional ones
+        for _i in 0..n {
+            me.table(RefType::FUNCREF, 2, None);
         }
         // wasmparser has an limit of 100 tables. wasmi does not have such a limit
         me.finish_no_validate()