From 9adfd9db240c08b8481f0aa5d69deca33c40180e Mon Sep 17 00:00:00 2001 From: xermicus Date: Wed, 8 Jul 2026 15:44:11 +0200 Subject: [PATCH] Fix/ny dynamic calldatacopy fmp (#564) Bug found by @jubnzv: `--newyork`: a `calldatacopy`/`codecopy`/`returndatacopy` to a dynamic destination that could overwrite the free-memory-pointer word truncated a subsequent `mload(0x40)` to `0`. --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Agent --- CHANGELOG.md | 1 + .../contracts/CalldatacopyFmpDynBug.yul | 23 +++ crates/integration/src/tests.rs | 30 ++++ crates/newyork/src/heap_opt.rs | 138 +++++++++++++++++- 4 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 crates/integration/contracts/CalldatacopyFmpDynBug.yul diff --git a/CHANGELOG.md b/CHANGELOG.md index 4724867..f36b47c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Supported `polkadot-sdk` rev: `2604.2.0` - ICE triggered by `--disable-solc-optimizer --newyork where inlined loops could emit an unterminated basic block in the inlined entrypoint. [#561](https://github.com/paritytech/revive/pull/561) - `--newyork`: dead-store elimination could drop a store that a later overlapping unaligned `mload` reads, returning zeroed memory instead of the stored bytes. - `--newyork`: an unaligned full-word `mload`/`mstore` overlapping a word kept native (little-endian) by the heap optimization returned byte-swapped bytes. +- `--newyork`: a `calldatacopy`/`codecopy`/`returndatacopy` to a dynamic destination that could overwrite the free-memory-pointer word truncated a subsequent `mload(0x40)` to `0`. [#564](https://github.com/paritytech/revive/pull/564) ## v1.3.0 diff --git a/crates/integration/contracts/CalldatacopyFmpDynBug.yul b/crates/integration/contracts/CalldatacopyFmpDynBug.yul new file mode 100644 index 0000000..4027ac0 --- /dev/null +++ b/crates/integration/contracts/CalldatacopyFmpDynBug.yul @@ -0,0 +1,23 @@ +/// Soundness PoC (newyork FMP range proof): a `calldatacopy` whose *dynamic* +/// destination can land on the free-memory-pointer word `[0x40, 0x60)` corrupts +/// the FMP, but was not detected as doing so. +/// +/// `calldatacopy(and(b, 0xFF), 0, 23)` copies 23 bytes to a destination that +/// ranges over `[0, 0xFF]`; with `b`'s low byte `0x40` it overwrites the FMP +/// word. A subsequent `mload(0x40)` reads the corrupted pointer, but newyork kept +/// its `FMP < heap_size` range proof (only *static* copy destinations flagged +/// `fmp_could_be_unbounded`) and truncated the value to `0`. EVM (and the stock +/// pipeline) return the copied bytes: 23 × 0x11 then 9 × 0x00. The fix flags a +/// dynamic copy destination that is not provably free-pointer-relative +/// (`add(mload(0x40), k)`, which is `>= 0x80`). +object "CalldatacopyFmpDynBug" { + code { datacopy(0, dataoffset("CalldatacopyFmpDynBug_deployed"), datasize("CalldatacopyFmpDynBug_deployed")) return(0, datasize("CalldatacopyFmpDynBug_deployed")) } + object "CalldatacopyFmpDynBug_deployed" { + code { + let b := calldataload(32) + calldatacopy(and(b, 0xFF), 0, 23) + mstore(0, mload(64)) + return(0, 32) + } + } +} diff --git a/crates/integration/src/tests.rs b/crates/integration/src/tests.rs index 9c4c161..be95e06 100644 --- a/crates/integration/src/tests.rs +++ b/crates/integration/src/tests.rs @@ -4769,6 +4769,36 @@ fn array_bounds_probe() { } } +/// Regression (newyork FMP range proof): a `calldatacopy` whose *dynamic* +/// destination can land on the free-memory-pointer word `[0x40, 0x60)` corrupts +/// the FMP, but only *static* copy destinations flagged `fmp_could_be_unbounded`. +/// `calldatacopy(and(b, 0xFF), 0, 23)` (with `b`'s low byte `0x40`) overwrites the +/// FMP; the subsequent `mload(0x40)` was truncated to `0` by the surviving range +/// proof instead of returning the copied bytes (23 × 0x11 then 9 × 0x00). The fix +/// flags a dynamic copy destination unless it is provably free-pointer-relative +/// (`add(mload(0x40), k)`, `>= 0x80`). Compared newyork-PVM vs solc-EVM. +#[test] +fn calldatacopy_dynamic_dest_fmp_corruption() { + let mut actions = instantiate_yul( + "contracts/CalldatacopyFmpDynBug.yul", + "CalldatacopyFmpDynBug", + ); + // word0: source bytes (0x11); word1: `b` with low byte 0x40 (the copy destination). + let mut data = vec![0x11u8; 32]; + let mut b = [0u8; 32]; + b[31] = 0x40; + data.extend_from_slice(&b); + actions.push(Call { + origin: TestAddress::Alice, + dest: TestAddress::Instantiated(0), + value: 0, + gas_limit: Some(GAS_LIMIT), + storage_deposit_limit: None, + data, + }); + run_differential(actions); +} + /// Regression (newyork dead-store elimination): a store read back by an /// intervening unaligned *overlapping* load must not be eliminated as dead. /// `mem_opt` marked a pending store read only on an exact-offset load, so diff --git a/crates/newyork/src/heap_opt.rs b/crates/newyork/src/heap_opt.rs index 6aabb8e..a1cf405 100644 --- a/crates/newyork/src/heap_opt.rs +++ b/crates/newyork/src/heap_opt.rs @@ -566,7 +566,7 @@ impl HeapAnalysis { size > 0 && address < 0x60 && address.saturating_add(size) > 0x40 } (Some(address), None) => (0x40..0x60).contains(&address), - _ => false, + (None, _) => !self.is_free_pointer_relative(destination.id.0), }; if covers_fmp { self.fmp_could_be_unbounded = true; @@ -585,6 +585,47 @@ impl HeapAnalysis { } } + /// Whether a dynamic memory destination is a free-memory-pointer-relative address, and so + /// provably cannot land on the FMP word `[0x40, 0x60)`. + /// + /// A copy to a dynamic destination (`calldatacopy(dst, _, _)` etc.) corrupts the FMP when `dst` + /// can equal an offset in `[0x40, 0x60)`. solc only ever copies to an FMP-relative address + /// (`add(mload(0x40), k)`), which is `>= 0x80` because the bounded FMP is `>= 0x80` — so it + /// cannot hit the slot. A destination that is *not* recognizably FMP-relative (e.g. + /// `and(calldataload(p), 0xff)`, which ranges over `[0, 0xff]`) can, and must flag + /// `fmp_could_be_unbounded` so the corrupted `mload(0x40)` skips the `FMP < heap_size` range + /// proof. Recognized as at-or-above the free pointer: `mload(0x40)`, a literal base `>= 0x60` + /// that fits in 64 bits (`mem_opt` constant-forwards `mload(0x40)` to its `0x80` literal), an + /// `add` with such an operand, and `Var` forwarding chains. (The adversarial `add(mload(0x40), + /// k)` / `add(0x80, k)` that wraps mod 2^256 back to `0x40` is the same solc-unreachable residual + /// as the dynamic full-word `MStore` gap; see the `fmp_could_be_unbounded` field docs.) + fn is_free_pointer_relative(&self, value_id: u32) -> bool { + const MAX_DEPTH: u32 = 32; + let mut current = value_id; + for _ in 0..MAX_DEPTH { + match self.value_expressions.get(¤t) { + Some(Expression::MLoad { offset, .. }) => { + return self.extract_static_offset(offset) == Some(0x40); + } + Some(Expression::Literal { value, .. }) => { + let digits = value.to_u64_digits(); + return digits.len() <= 1 && digits.first().copied().unwrap_or(0) >= 0x60; + } + Some(Expression::Var(inner)) => current = inner.0, + Some(Expression::Binary { + operation: crate::ir::BinaryOperation::Add, + lhs, + rhs, + }) => { + return self.is_free_pointer_relative(lhs.id.0) + || self.is_free_pointer_relative(rhs.id.0); + } + _ => return false, + } + } + false + } + /// Taints every word a static, non-word-aligned full-word access (`mload`/`mstore`) covers. /// /// EVM memory is big-endian; the native-mode optimization keeps a word's bytes little-endian @@ -1205,6 +1246,18 @@ mod tests { use super::*; use num::BigUint; + /// Builds a `Statement::Let` binding `id` to the literal `value`. + fn literal(id: u32, value: u64) -> Statement { + use crate::ir::{Type, ValueId}; + Statement::Let { + bindings: vec![ValueId(id)], + value: Expression::Literal { + value: BigUint::from(value), + value_type: Type::default(), + }, + } + } + #[test] fn test_alignment_computation() { assert_eq!(compute_alignment(0), 32); @@ -1238,6 +1291,89 @@ mod tests { assert_eq!(gcd(0, 5), 5); } + /// Builds an object whose runtime code binds `dest` (id 10) and copies 23 + /// bytes of calldata to it, then reads `mload(0x40)`. + fn object_with_dynamic_copy(dest_setup: Vec) -> Object { + use crate::ir::{Block, ValueId}; + let mut statements = dest_setup; + statements.push(literal(20, 0)); + statements.push(literal(21, 23)); + statements.push(Statement::CallDataCopy { + destination: Value::int(ValueId(10)), + offset: Value::int(ValueId(20)), + length: Value::int(ValueId(21)), + }); + Object { + name: "T".to_string(), + code: Block { statements }, + functions: std::collections::BTreeMap::new(), + subobjects: vec![], + data: std::collections::BTreeMap::new(), + } + } + + /// A dynamic copy destination that is not provably free-pointer-relative + /// (`and(calldataload(p), 0xff)`, range `[0, 0xff]`) can land on the FMP word, + /// so it must flag the FMP as possibly unbounded. + #[test] + fn dynamic_copy_non_fmp_dest_flags_unbounded() { + use crate::ir::{BinaryOperation, ValueId}; + let dest_setup = vec![ + Statement::Let { + bindings: vec![ValueId(1)], + value: Expression::CallDataLoad { + offset: Value::int(ValueId(0)), + }, + }, + literal(2, 0xff), + Statement::Let { + bindings: vec![ValueId(10)], + value: Expression::Binary { + operation: BinaryOperation::And, + lhs: Value::int(ValueId(1)), + rhs: Value::int(ValueId(2)), + }, + }, + ]; + let results = object_with_dynamic_copy(dest_setup).analyze_heap(); + assert!( + results.fmp_could_be_unbounded(), + "a masked calldata destination can hit the FMP word" + ); + } + + /// A dynamic copy destination that is free-pointer-relative + /// (`add(mload(0x40), 0x20)`, `>= 0x80`) cannot hit the FMP word, so it must + /// not disable the FMP optimization. + #[test] + fn dynamic_copy_fmp_relative_dest_stays_bounded() { + use crate::ir::{BinaryOperation, ValueId}; + let dest_setup = vec![ + literal(0, 0x40), + Statement::Let { + bindings: vec![ValueId(1)], + value: Expression::MLoad { + offset: Value::int(ValueId(0)), + region: MemoryRegion::FreePointerSlot, + }, + }, + literal(2, 0x20), + Statement::Let { + bindings: vec![ValueId(10)], + value: Expression::Binary { + operation: BinaryOperation::Add, + lhs: Value::int(ValueId(1)), + rhs: Value::int(ValueId(2)), + }, + }, + ]; + let results = object_with_dynamic_copy(dest_setup).analyze_heap(); + assert!( + !results.fmp_could_be_unbounded(), + "an add(mload(0x40), k) destination is >= 0x80 and cannot hit the FMP word" + ); + } + #[test] fn test_offset_info_from_literal() { let analysis = HeapAnalysis::new();