From f04386dcf7153fe4afabc324ab46f1b1fb1b0a50 Mon Sep 17 00:00:00 2001 From: Timothy Banks Date: Thu, 24 Sep 2026 10:57:02 -0400 Subject: [PATCH] fix: Pin dynamic fuel costs in wasmi engine (#8259) --- crates/xrpl-wasm-vm/src/vm.rs | 57 ++++++++++++++++++++++++- crates/xrpl-wasm-vm/tests/vm_limits.rs | 58 +++++++++++++++++++------- src/benchmarks/libxrpl/wasm/README.md | 21 +++++++--- src/benchmarks/libxrpl/wasm/Vm.cpp | 35 ++++++++++++---- 4 files changed, 142 insertions(+), 29 deletions(-) diff --git a/crates/xrpl-wasm-vm/src/vm.rs b/crates/xrpl-wasm-vm/src/vm.rs index d35032f1a6..345cf8995a 100644 --- a/crates/xrpl-wasm-vm/src/vm.rs +++ b/crates/xrpl-wasm-vm/src/vm.rs @@ -1,8 +1,8 @@ use std::cell::Cell; use std::fmt; use wasmi::{ - CompilationMode, Config, Engine, Export, Linker, Memory, Module, Store, StoreLimits, - StoreLimitsBuilder, TrapCode, + CompilationMode, Config, CustomFuelCosts, EnforcedLimits, Engine, Export, Linker, Memory, + Module, Store, StoreLimits, StoreLimitsBuilder, TrapCode, }; use xrpl_host_functions::HostFunctions; @@ -262,6 +262,17 @@ pub(crate) fn wasm_engine() -> Engine { // config.wasm_memory64(false); config.wasm_wide_arithmetic(false); config.allow_start_fn(false); + config.enforced_limits(EnforcedLimits::strict()); + + let fuel_costs = CustomFuelCosts { + bytes_copied_per_fuel: 64, + fuel_per_bytes_translated: 7, + fuel_per_bytes_validated: 2, + }; + config.fuel_cost(fuel_costs); + // config.operator_costs is already guarded by the probe_fuel test under budgets.rs + // in that a change to operator costs in a future version will be a loud failure. + config.compilation_mode(CompilationMode::LazyTranslation); Engine::new(&config) } @@ -396,6 +407,48 @@ mod tests { assert_eq!(limits.memories(), 1); } + /// The three size-proportional fuel rates, read back off the engine. wasmi + /// takes its own defaults for these unless told otherwise, so an upgrade that + /// changed one would retune our gas silently. `Config` keeps them + /// `pub(crate)` and exposes them only through `Debug`. + #[test] + fn the_dynamic_fuel_costs_are_pinned() { + let config = format!("{:?}", wasm_engine().config()); + for rate in [ + "bytes_copied_per_fuel: 64", + "fuel_per_bytes_translated: 7", + "fuel_per_bytes_validated: 2", + ] { + assert!(config.contains(rate), "expected `{rate}` in {config}"); + } + } + + /// [`EnforcedLimits::strict`] is the one line in [`wasm_engine`] that takes a + /// value rather than stating one — the fields are `pub(crate)`, so the preset is + /// the only way to set them. + #[test] + fn the_enforced_limits_are_pinned() { + const EXPECTED: &str = concat!( + "EnforcedLimits { ", + "max_globals: Some(1000), ", + "max_functions: Some(10000), ", + "max_tables: Some(100), ", + "max_element_segments: Some(1000), ", + "max_memories: Some(1), ", + "max_data_segments: Some(1000), ", + "max_params: Some(32), ", + "max_results: Some(32), ", + "min_avg_bytes_per_function: Some(AvgBytesPerFunctionLimit { ", + "req_funcs_bytes: 1000, min_avg_bytes_per_function: 40 }) }", + ); + + let config = format!("{:?}", wasm_engine().config()); + assert!( + config.contains(EXPECTED), + "expected `{EXPECTED}` in {config}" + ); + } + /// The only place these numbers appear as literals; every other test derives /// them from the constants. #[test] diff --git a/crates/xrpl-wasm-vm/tests/vm_limits.rs b/crates/xrpl-wasm-vm/tests/vm_limits.rs index 016b35949c..a117a528c6 100644 --- a/crates/xrpl-wasm-vm/tests/vm_limits.rs +++ b/crates/xrpl-wasm-vm/tests/vm_limits.rs @@ -168,7 +168,8 @@ fn a_declared_table_maximum_past_the_cap_is_allowed_but_unreachable() { /// One row per feature `wasm_engine` turns off: the smallest module that uses /// it, and the fragment of wasmi's refusal that names the feature. A row declaring /// its own memory omits [`ONE_PAGE`], or it is refused for having two memories -/// instead. +/// instead — except `wasm_multi_memory`, where two memories are the point and one +/// of them has to be imported to reach the feature check at all. fn disabled_features() -> Vec<(&'static str, Vec<&'static str>, &'static str, &'static str)> { vec![ ( @@ -223,9 +224,14 @@ fn disabled_features() -> Vec<(&'static str, Vec<&'static str>, &'static str, &' "(global.get $g)", "non-constant operator", ), + // One memory imported, one defined. `EnforcedLimits::strict()` caps memories + // at one and checks the memory *section* before the validator sees it + // (`module/parser/mod.rs`, `process_memories`), so two *defined* memories are + // refused for exceeding the cap and never reach the feature check. An import + // is not in that section, so this is the shape that names the proposal. ( "wasm_multi_memory", - vec![ONE_PAGE, "(memory 1)"], + vec![r#"(import "host_lib" "mem" (memory 1))"#, ONE_PAGE], "(i32.const 0)", "multiple memories", ), @@ -278,7 +284,7 @@ fn every_disabled_feature_is_refused_by_name() { } } -/// The three knobs [`every_disabled_feature_is_refused_by_name`] cannot cover. The +/// The knobs [`every_disabled_feature_is_refused_by_name`] cannot cover. The /// configuration is the same for every engine `wasm_engine` builds, so a test /// observes the one `wasm_engine` makes: a knob masked by another, or with no /// caller-visible effect, has no distinguishing module. @@ -293,6 +299,14 @@ fn the_knobs_without_a_module_of_their_own() { assert!(refusal.contains("floating-point"), "{refusal}"); assert!(!refusal.contains("saturating"), "{refusal}"); + // `EnforcedLimits::strict()`'s `max_memories: Some(1)`, which masks + // `wasm_multi_memory(false)` for every module that defines its two memories + // rather than importing one — the case a real contract would hit. The feature + // flag itself is covered by name in [`disabled_features`]. + let wat = module(&[ONE_PAGE, "(memory 1)"], "(i32.const 0)"); + let refusal = assert_stage!(failure(&wat, &host), RunError::Compile(_)).to_string(); + assert!(refusal.contains("limit of 1 memories"), "{refusal}"); + // `ignore_custom_sections(true)`: governs whether wasmi retains custom // sections, not accept/reject, so this pins only that one is harmless. let wat = module( @@ -647,20 +661,36 @@ fn unbounded_recursion_is_stopped_by_the_call_stack_limit() { assert_stage!(failure(&wat, &host), RunError::Trap(_)); } -/// A module with many functions currently compiles and runs: wasmi's only cap is its -/// 1,000,000 hard limit. +/// The CodeMap-DoS defense, from the guest's side: a module of thousands of tiny +/// functions is refused in `Module::new`, before anything is translated. +/// +/// Both of `EnforcedLimits::strict()`'s function rules are load-bearing here, which +/// is why the second half exists — dropping under the count cap does not get a +/// module past the defense, because the bodies then fail the minimum average. The +/// values themselves are pinned in `vm.rs`'s `the_enforced_limits_are_pinned`. #[test] -#[ignore = "CodeMap-DoS unmitigated; a function-count limit is deferred to preflight parsing"] -fn many_functions_currently_run_unbounded() { +fn a_module_of_too_many_functions_is_refused() { let host = FakeHost::new(); - let funcs: String = (0..24_000) - .map(|i| format!("(func $f{i} (result i32) (i32.const {}))", i % 7)) - .collect(); - let wat = - format!("(module {ONE_PAGE} {funcs} (func (export \"finish\") (result i32) (call $f0)))"); + + let tiny_funcs = |count: usize| { + let funcs: String = (0..count) + .map(|i| format!("(func $f{i} (result i32) (i32.const {}))", i % 7)) + .collect(); + format!("(module {ONE_PAGE} {funcs} (func (export \"finish\") (result i32) (call $f0)))") + }; + + // `max_functions: Some(10000)`, checked before the bodies are looked at. + let wat = tiny_funcs(10_001); + let refusal = assert_stage!(failure(&wat, &host), RunError::Compile(_)).to_string(); + assert!(refusal.contains("limit of 10000 functions"), "{refusal}"); + + // `min_avg_bytes_per_function: 40`, enforced once the bodies total 1 KiB. These + // average five bytes, so the count cap is not the only thing holding. + let wat = tiny_funcs(9_999); + let refusal = assert_stage!(failure(&wat, &host), RunError::Compile(_)).to_string(); assert!( - run(&wat, &host).is_ok(), - "a large-function module currently compiles and runs" + refusal.contains("minimum average bytes per function of 40"), + "{refusal}" ); } diff --git a/src/benchmarks/libxrpl/wasm/README.md b/src/benchmarks/libxrpl/wasm/README.md index ce1abbb6fe..7edc9d9504 100644 --- a/src/benchmarks/libxrpl/wasm/README.md +++ b/src/benchmarks/libxrpl/wasm/README.md @@ -120,13 +120,22 @@ appearing a second time, because the transactor validates and executes with no m them. The size sweeps matter more than the floor: a fixed cost is only a griefing concern if it is large, but a slope against attacker-chosen module size is one at any height. -Two caveats when reading a sweep. `gas_per_byte` is an **average** carrying the case's fixed cost, +One caveat when reading a sweep: `gas_per_byte` is an **average** carrying the case's fixed cost, not a marginal rate — it overestimates, and falls toward the true slope as the module grows, so read -the convergence rather than any single row. And the `/4096` points get few iterations and go noisy -first; compare `rel_error` across the sweep before quoting the largest one. +the convergence rather than any single row. A quiet Release run converges 31.4 → 13.2 → 7.8 → 6.7 → +6.5 across `compileScaling`. -The sweep stops at 4096 functions for want of a real cap to stop at — no maximum contract size is -enforced anywhere yet, the transactor not being wired. +**The filler modules are shaped by the engine's limits, not chosen freely.** +`EnforcedLimits::strict()` refuses any module averaging under 40 bytes per function body once bodies +total 1 KiB — a limit wasmi added to defend lazy compilation against precisely the shape a size +sweep wants. So `fillerWat` gives each body `kFillerChain` mul/add pairs to clear that floor; one +pair averages 12 bytes and is refused outright. Thinning the bodies to get more functions per byte +does not make a harder module, it makes an inadmissible one. + +The sweep tops out at 2048 functions, bounded by **bytes** rather than function count: ~158 KiB +against `kMaxBytecodeSizeLimit` of 200,000, where 4096 functions would be ~317 KiB and refused +earlier in the transactor. `strict()`'s own `max_functions` of 10,000 never binds — 40 bytes per +function against a 200,000-byte module caps any admissible contract at 5,000. The linker rebuild and the fuel-metering overhead are **not** separable from here — C++ sees only `runEscrowWasm` and `preflightEscrowWasm`. Both need benchmarks inside `xrpl-wasm-vm`, where @@ -135,7 +144,7 @@ The linker rebuild and the fuel-metering overhead are **not** separable from her ### Pin your iteration counts Pin `->Iterations(...)`: automatic sizing targets a wall-clock budget, not a compile count, -so the cheap cases get six-figure counts and `/4096` a handful — leaving no row comparable to +so the cheap cases get six-figure counts and `/2048` a handful — leaving no row comparable to another or to the last run. ## Gotchas, each of which has already cost someone an afternoon diff --git a/src/benchmarks/libxrpl/wasm/Vm.cpp b/src/benchmarks/libxrpl/wasm/Vm.cpp index 353bd8a1b6..4cd4fab2b4 100644 --- a/src/benchmarks/libxrpl/wasm/Vm.cpp +++ b/src/benchmarks/libxrpl/wasm/Vm.cpp @@ -15,7 +15,7 @@ namespace { // README.md has what each case measures and how to read `gas_equivalent`. // // **Every case pins `->Iterations(...)`.** Automatic sizing targets a wall-clock budget rather than -// a compile count, so it gives the cheap cases six-figure counts and `/4096` a handful, leaving the +// a compile count, so it gives the cheap cases six-figure counts and `/2048` a handful, leaving the // sweep's rows incomparable. // The smallest module the engine accepts. Everything a run does to it is overhead by construction. @@ -29,6 +29,24 @@ minimalWat() )wat"; } +// How many mul/add pairs each filler body holds. +// +// **Not a tuning knob — a floor set by the engine.** `EnforcedLimits::strict()` refuses any module +// averaging under 40 bytes per function body, once bodies total 1 KiB. That limit exists to defend +// lazy compilation against exactly the shape this function generates, so a body has to be fat +// enough to be a module the engine would actually accept. One pair averages 12 bytes and is +// refused; four averages 35 and is still refused; six is the first that passes. Eight is used for +// margin, and lands around 60 bytes per function. +constexpr size_t kFillerChain = 8; + +// The top of the size sweeps. +// +// Bounded by bytes rather than by function count: at ~77 bytes per function this is ~158 KiB, +// inside `kMaxBytecodeSizeLimit` (200,000), while 4096 functions would be ~317 KiB and past it. +// `EnforcedLimits::strict()`'s own `max_functions` of 10,000 never binds — 40 bytes per function +// minimum against a 200,000-byte module caps any accepted contract at 5,000 functions. +constexpr size_t kFillerMaxFunctions = 2048; + // `count` unreachable functions on top of the minimal module: bigger without doing more. // // Each body is seeded with its own index so no two are identical and none folds to a constant the @@ -41,10 +59,13 @@ fillerWat(size_t count) auto out = std::string{"(module\n (memory (export \"memory\") 1)\n"}; for (auto i = 0uz; i < count; ++i) { - out += std::format( - " (func $f{0} (param i32) (result i32)\n" - " (i32.add (i32.mul (local.get 0) (i32.const {0})) (i32.const {0})))\n", - i); + out += std::format(" (func $f{} (param i32) (result i32)\n (local.get 0)\n", i); + for (auto k = 0uz; k < kFillerChain; ++k) + { + out += std::format( + " (i32.mul (i32.const {})) (i32.add (i32.const {}))\n", i + k + 1, i + k + 2); + } + out += " )\n"; } out += " (func (export \"escrow_finish\") (result i32)\n (i32.const 1)))\n"; return out; @@ -113,7 +134,7 @@ BENCHMARK(compileScaling) ->Arg(8) ->Arg(64) ->Arg(512) - ->Arg(4096); + ->Arg(kFillerMaxFunctions); void runScaling(benchmark::State& state) @@ -128,7 +149,7 @@ BENCHMARK(runScaling) ->Arg(8) ->Arg(64) ->Arg(512) - ->Arg(4096); + ->Arg(kFillerMaxFunctions); void instantiateScaling(benchmark::State& state)