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) <noreply@anthropic.com>
Co-authored-by: Agent <agent@local>

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
 

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)
+    }
+  }
+}

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

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(&current) {
+                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<Statement>) -> 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();