Clip OOB values in call data load and copy (#448)

The OOB logic is implemented in the runtime but truncation circumvented
it.

Closes #263

---------

Signed-off-by: xermicus <cyrill@parity.io>
Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

CHANGELOG.md

@@ -6,6 +6,9 @@ This is a development pre-release.
 
 Supported `polkadot-sdk` rev: `unstable2507`
 
+### Fixed
+- OOB access in `calldataload` and `calldatacopy` should always produce zero values instead of consuming all gas.
+
 ## v0.6.0
 
 This is a development pre-release.

crates/integration/codesize.json

@@ -1,10 +1,10 @@
 {
-  "Baseline": 911,
+  "Baseline": 864,
-  "Computation": 2337,
+  "Computation": 2323,
-  "DivisionArithmetics": 14488,
+  "DivisionArithmetics": 14475,
-  "ERC20": 17189,
+  "ERC20": 17229,
-  "Events": 1672,
+  "Events": 1663,
-  "FibonacciIterative": 1454,
+  "FibonacciIterative": 1421,
-  "Flipper": 2106,
+  "Flipper": 2154,
-  "SHA1": 7814
+  "SHA1": 7751
 }

crates/integration/contracts/MemoryBounds.sol

@@ -21,12 +21,43 @@ pragma solidity ^0.8.24;
           "Instantiated": 0
         }
       }
+    },
+    {
+      "Call": {
+        "dest": {
+          "Instantiated": 0
+        },
+        "data": "489eb33e00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001a300000000000000000000000000000000000000000000000000000000000000"
+      }
+    },
+    {
+      "Call": {
+        "dest": {
+          "Instantiated": 0
+        },
+        "data": "db1ff7bb00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000001a300000000000000000000000000000000000000000000000000000000000000"
+      }
     }
   ]
 }
 */
 
 contract MemoryBounds {
+    function oobLoad(bytes memory data) public pure returns (uint256, bytes memory) {
+        uint256 result1;
+        assembly {
+            let ptr := mload(add(data, 0x20))
+            result1 := calldataload(ptr)
+        }
+        return (result1, data);
+    }
+
+    function oobCopy(bytes memory data) public pure returns (uint256, bytes memory) {
+        assembly {
+            calldatacopy(0, 0x3a0000000000000000000000000000000000, 0x20)
+        }
+    }
+
     fallback() external {
         assembly {
             // Accessing OOB offsets should always work when the length is 0.

crates/llvm-context/src/polkavm/context/mod.rs

@@ -1054,6 +1054,28 @@ impl<'ctx> Context<'ctx> {
             .into_int_value())
     }
 
+    /// Clip a memory offset to the maximum value that fits into a register.
+    pub fn clip_to_xlen(
+        &self,
+        value: inkwell::values::IntValue<'ctx>,
+    ) -> anyhow::Result<inkwell::values::IntValue<'ctx>> {
+        let clipped = self.xlen_type().const_all_ones();
+        let is_overflow = self.builder().build_int_compare(
+            inkwell::IntPredicate::UGT,
+            value,
+            self.builder()
+                .build_int_z_extend(clipped, self.word_type(), "value_clipped")?,
+            "is_value_overflow",
+        )?;
+        let truncated =
+            self.builder()
+                .build_int_truncate(value, self.xlen_type(), "value_truncated")?;
+        Ok(self
+            .builder()
+            .build_select(is_overflow, clipped, truncated, "value")?
+            .into_int_value())
+    }
+
     /// Build a call to PolkaVM `sbrk` for extending the heap from offset by `size`.
     /// The allocation is aligned to 32 bytes.
     ///

crates/llvm-context/src/polkavm/evm/calldata.rs

@@ -8,7 +8,7 @@ pub fn load<'ctx>(
     offset: inkwell::values::IntValue<'ctx>,
 ) -> anyhow::Result<inkwell::values::BasicValueEnum<'ctx>> {
     let output_pointer = context.build_alloca_at_entry(context.word_type(), "call_data_output");
-    let offset = context.safe_truncate_int_to_xlen(offset)?;
+    let offset = context.clip_to_xlen(offset)?;
 
     context.build_runtime_call(
         revive_runtime_api::polkavm_imports::CALL_DATA_LOAD,
@@ -40,9 +40,9 @@ pub fn copy<'ctx>(
     source_offset: inkwell::values::IntValue<'ctx>,
     size: inkwell::values::IntValue<'ctx>,
 ) -> anyhow::Result<()> {
-    let source_offset = context.safe_truncate_int_to_xlen(source_offset)?;
+    let source_offset = context.clip_to_xlen(source_offset)?;
-    let size = context.safe_truncate_int_to_xlen(size)?;
+    let size = context.clip_to_xlen(size)?;
-    let destination_offset = context.safe_truncate_int_to_xlen(destination_offset)?;
+    let destination_offset = context.clip_to_xlen(destination_offset)?;
     let output_pointer = context.build_heap_gep(destination_offset, size)?;
 
     context.build_runtime_call(