Bug: fix to truncate memory offset of Word to Uint64 when memory leng…

…th is zero (scroll-tech#393)

* Truncate memory offset of Word to Uint64 for Call opcodes when memory length is zero.

* Truncate memory offset for `CALLDATACOPY`.

* Truncate memory offset for `CODECOPY`.

* Truncate memory offset for `EXTCODECOPY`.

* Truncate memory offset for `RETURNDATACOPY`.

* Truncate memory offset for `RETURN` and `REVERT`.

* Truncate memory offset for `SHA3` (`KECCAK256`).

* Truncate memory offset for `CREATE` (and `CREATE2`).

bus-mapping/src/circuit_input_builder.rs

@@ -685,10 +685,17 @@ pub fn get_create_init_code<'a>(
     call_ctx: &'a CallContext,
     step: &GethExecStep,
 ) -> Result<&'a [u8], Error> {
-    let offset = step.stack.nth_last(1)?;
-    let length = step.stack.nth_last(2)?;
-    Ok(&call_ctx.memory.0
-        [offset.low_u64() as usize..(offset.low_u64() + length.low_u64()) as usize])
+    let offset = step.stack.nth_last(1)?.low_u64() as usize;
+    let length = step.stack.nth_last(2)?.as_usize();
+
+    let mem_len = call_ctx.memory.0.len();
+    if offset >= mem_len {
+        return Ok(&[]);
+    }
+
+    let offset_end = offset.checked_add(length).unwrap_or(mem_len);
+
+    Ok(&call_ctx.memory.0[offset..offset_end])
 }
 
 /// Retrieve the memory offset and length of call.

bus-mapping/src/evm/opcodes/calldatacopy.rs

@@ -102,7 +102,8 @@ fn gen_copy_event(
     let call_data_offset = state.call()?.call_data_offset;
     let call_data_length = state.call()?.call_data_length;
 
-    let dst_addr = memory_offset.as_u64();
+    // Get low Uint64 of offset.
+    let dst_addr = memory_offset.low_u64();
     let src_addr_end = call_data_offset.checked_add(call_data_length).unwrap();
 
     // Reset offset to call_data_length if overflow, and set source start to the

bus-mapping/src/evm/opcodes/callop.rs

@@ -27,9 +27,9 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
         let geth_step = &geth_steps[0];
         let mut exec_step = state.new_step(geth_step)?;
 
-        let args_offset = geth_step.stack.nth_last(N_ARGS - 4)?.as_usize();
+        let args_offset = geth_step.stack.nth_last(N_ARGS - 4)?.low_u64() as usize;
         let args_length = geth_step.stack.nth_last(N_ARGS - 3)?.as_usize();
-        let ret_offset = geth_step.stack.nth_last(N_ARGS - 2)?.as_usize();
+        let ret_offset = geth_step.stack.nth_last(N_ARGS - 2)?.low_u64() as usize;
         let ret_length = geth_step.stack.nth_last(N_ARGS - 1)?.as_usize();
 
         // we need to keep the memory until parse_call complete
@@ -255,7 +255,11 @@ impl<const N_ARGS: usize> Opcode for CallOpcode<N_ARGS> {
                 let code_address = code_address.unwrap();
                 let (result, contract_gas_cost) = execute_precompiled(
                     &code_address,
-                    &caller_ctx.memory.0[args_offset..args_offset + args_length],
+                    if args_length != 0 {
+                        &caller_ctx.memory.0[args_offset..args_offset + args_length]
+                    } else {
+                        &[]
+                    },
                     callee_gas_left,
                 );
                 log::trace!(

bus-mapping/src/evm/opcodes/codecopy.rs

@@ -78,7 +78,8 @@ fn gen_copy_event(
     let bytecode: Bytecode = state.code(code_hash)?.into();
     let code_size = bytecode.code.len() as u64;
 
-    let dst_addr = dst_offset.as_u64();
+    // Get low Uint64 of offset.
+    let dst_addr = dst_offset.low_u64();
     let src_addr_end = code_size;
 
     // Reset offset to Uint64 maximum value if overflow, and set source start to the

bus-mapping/src/evm/opcodes/create.rs

@@ -20,7 +20,8 @@ impl<const IS_CREATE2: bool> Opcode for Create<IS_CREATE2> {
         let geth_step = &geth_steps[0];
         let mut exec_step = state.new_step(geth_step)?;
 
-        let offset = geth_step.stack.nth_last(1)?.as_usize();
+        // Get low Uint64 of offset.
+        let offset = geth_step.stack.nth_last(1)?.low_u64() as usize;
         let length = geth_step.stack.nth_last(2)?.as_usize();
 
         if length != 0 {

bus-mapping/src/evm/opcodes/extcodecopy.rs

@@ -135,7 +135,8 @@ fn gen_copy_event(
     };
     let code_size = bytecode.code.len() as u64;
 
-    let dst_addr = dst_offset.as_u64();
+    // Get low Uint64 of offset.
+    let dst_addr = dst_offset.low_u64();
     let src_addr_end = code_size;
 
     // Reset offset to Uint64 maximum value if overflow, and set source start to the

bus-mapping/src/evm/opcodes/return_revert.rs

@@ -43,7 +43,8 @@ impl Opcode for ReturnRevert {
             call.is_success.to_word(),
         );
 
-        let offset = offset.as_usize();
+        // Get low Uint64 of offset.
+        let offset = offset.low_u64() as usize;
         let length = length.as_usize();
 
         // Case A in the spec.

bus-mapping/src/evm/opcodes/returndatacopy.rs

@@ -121,7 +121,8 @@ fn gen_copy_event(
     state: &mut CircuitInputStateRef,
     geth_step: &GethExecStep,
 ) -> Result<CopyEvent, Error> {
-    let dst_addr = geth_step.stack.nth_last(0)?.as_u64();
+    // Get low Uint64 of offset.
+    let dst_addr = geth_step.stack.nth_last(0)?.low_u64();
     let data_offset = geth_step.stack.nth_last(1)?.as_u64();
     let length = geth_step.stack.nth_last(2)?.as_u64();
 

bus-mapping/src/evm/opcodes/sha3.rs

@@ -40,7 +40,7 @@ impl Opcode for Sha3 {
         let memory = state
             .call_ctx()?
             .memory
-            .read_chunk(offset.as_usize().into(), size.as_usize().into());
+            .read_chunk(offset.low_u64().into(), size.as_usize().into());
 
         // keccak-256 hash of the given data in memory.
         let sha3 = keccak256(&memory);
@@ -65,8 +65,11 @@ impl Opcode for Sha3 {
         state.push_copy(
             &mut exec_step,
             CopyEvent {
-                src_addr: offset.as_u64(),
-                src_addr_end: offset.as_u64() + size.as_u64(),
+                src_addr: offset.low_u64(),
+                src_addr_end: offset
+                    .low_u64()
+                    .checked_add(size.as_u64())
+                    .unwrap_or(u64::MAX),
                 src_type: CopyDataType::Memory,
                 src_id: NumberOrHash::Number(call_id),
                 dst_addr: 0,

eth-types/src/evm_types/memory.rs

@@ -332,10 +332,12 @@ impl Memory {
         // Reference go-ethereum `opCallDataCopy` function for defails.
         // https://github.com/ethereum/go-ethereum/blob/bb4ac2d396de254898a5f44b1ea2086bfe5bd193/core/vm/instructions.go#L299
 
-        // Both `dst_offset` and `length` should be checked for overflow during gas cost
-        // calculation. Otherwise should return an out of gas error previously.
+        // `length` should be checked for overflow during gas cost calculation.
+        // Otherwise should return an out of gas error previously.
         let length = length.as_usize();
         if length != 0 {
+            // `dst_offset` should be within range if length is non-zero.
+            // https://github.com/ethereum/go-ethereum/blob/bb4ac2d396de254898a5f44b1ea2086bfe5bd193/core/vm/common.go#L37
             let dst_offset = dst_offset.as_u64();
 
             // Reset data offset to the maximum value of Uint64 if overflow.

zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs

@@ -260,9 +260,9 @@ mod test {
 
     fn test_ok_root(
         call_data_length: usize,
-        memory_offset: usize,
         length: usize,
         data_offset: Word,
+        memory_offset: Word,
     ) {
         let bytecode = bytecode! {
             PUSH32(length)
@@ -299,9 +299,9 @@ mod test {
     fn test_ok_internal(
         call_data_offset: usize,
         call_data_length: usize,
-        dst_offset: usize,
         length: usize,
         data_offset: Word,
+        dst_offset: Word,
     ) {
         let (addr_a, addr_b) = (mock::MOCK_ACCOUNTS[0], mock::MOCK_ACCOUNTS[1]);
 
@@ -354,31 +354,37 @@ mod test {
 
     #[test]
     fn calldatacopy_gadget_simple() {
-        test_ok_root(0x40, 0x40, 10, 0x00.into());
-        test_ok_internal(0x40, 0x40, 0xA0, 10, 0x10.into());
+        test_ok_root(0x40, 10, 0x00.into(), 0x40.into());
+        test_ok_internal(0x40, 0x40, 10, 0x10.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_large() {
-        test_ok_root(0x204, 0x103, 0x101, 0x102.into());
-        test_ok_internal(0x30, 0x204, 0x103, 0x101, 0x102.into());
+        test_ok_root(0x204, 0x101, 0x102.into(), 0x103.into());
+        test_ok_internal(0x30, 0x204, 0x101, 0x102.into(), 0x103.into());
     }
 
     #[test]
     fn calldatacopy_gadget_out_of_bound() {
-        test_ok_root(0x40, 0x40, 40, 0x20.into());
-        test_ok_internal(0x40, 0x20, 0xA0, 10, 0x28.into());
+        test_ok_root(0x40, 40, 0x20.into(), 0x40.into());
+        test_ok_internal(0x40, 0x20, 10, 0x28.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_zero_length() {
-        test_ok_root(0x40, 0x40, 0, 0x00.into());
-        test_ok_internal(0x40, 0x40, 0xA0, 0, 0x10.into());
+        test_ok_root(0x40, 0, 0x00.into(), 0x40.into());
+        test_ok_internal(0x40, 0x40, 0, 0x10.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_data_offset_overflow() {
-        test_ok_root(0x40, 0x40, 0, Word::MAX);
-        test_ok_internal(0x40, 0x40, 0xA0, 0, Word::MAX);
+        test_ok_root(0x40, 10, Word::MAX, 0x40.into());
+        test_ok_internal(0x40, 0x40, 10, Word::MAX, 0xA0.into());
+    }
+
+    #[test]
+    fn calldatacopy_gadget_overflow_memory_offset_and_zero_length() {
+        test_ok_root(0x40, 0, 0x40.into(), Word::MAX);
+        test_ok_internal(0x40, 0x40, 0, 0x10.into(), Word::MAX);
     }
 }

zkevm-circuits/src/evm_circuit/execution/callop.rs

@@ -879,31 +879,31 @@ mod test {
             },
             // With memory expansion
             Stack {
-                cd_offset: 64,
+                cd_offset: 64.into(),
                 cd_length: 320,
-                rd_offset: 0,
+                rd_offset: Word::zero(),
                 rd_length: 32,
                 ..Default::default()
             },
             Stack {
-                cd_offset: 0,
+                cd_offset: Word::zero(),
                 cd_length: 32,
-                rd_offset: 64,
+                rd_offset: 64.into(),
                 rd_length: 320,
                 ..Default::default()
             },
             Stack {
-                cd_offset: 0xFFFFFF,
+                cd_offset: 0xFFFFFF.into(),
                 cd_length: 0,
-                rd_offset: 0xFFFFFF,
+                rd_offset: 0xFFFFFF.into(),
                 rd_length: 0,
                 ..Default::default()
             },
             // With memory expansion and value
             Stack {
-                cd_offset: 64,
+                cd_offset: 64.into(),
                 cd_length: 320,
-                rd_offset: 0,
+                rd_offset: 0.into(),
                 rd_length: 32,
                 value: Word::from(10).pow(18.into()),
                 ..Default::default()
@@ -921,13 +921,28 @@ mod test {
             });
     }
 
+    #[test]
+    fn callop_overflow_offset_and_zero_length() {
+        let stack = Stack {
+            cd_offset: Word::MAX,
+            cd_length: 0,
+            rd_offset: Word::MAX,
+            rd_length: 0,
+            ..Default::default()
+        };
+
+        TEST_CALL_OPCODES
+            .iter()
+            .for_each(|opcode| test_ok(caller(opcode, stack, true), callee(bytecode! {})));
+    }
+
     #[derive(Clone, Copy, Debug, Default)]
     struct Stack {
         gas: u64,
         value: Word,
-        cd_offset: u64,
+        cd_offset: Word,
         cd_length: u64,
-        rd_offset: u64,
+        rd_offset: Word,
         rd_length: u64,
     }
 
@@ -954,9 +969,9 @@ mod test {
         // Call twice for testing both cold and warm access
         let mut bytecode = bytecode! {
             PUSH32(Word::from(stack.rd_length))
-            PUSH32(Word::from(stack.rd_offset))
+            PUSH32(stack.rd_offset)
             PUSH32(Word::from(stack.cd_length))
-            PUSH32(Word::from(stack.cd_offset))
+            PUSH32(stack.cd_offset)
         };
         if is_call_or_callcode {
             bytecode.push(32, stack.value);
@@ -966,9 +981,9 @@ mod test {
             PUSH32(Word::from(stack.gas))
             .write_op(*opcode)
             PUSH32(Word::from(stack.rd_length))
-            PUSH32(Word::from(stack.rd_offset))
+            PUSH32(stack.rd_offset)
             PUSH32(Word::from(stack.cd_length))
-            PUSH32(Word::from(stack.cd_offset))
+            PUSH32(stack.cd_offset)
         });
         if is_call_or_callcode {
             bytecode.push(32, stack.value);
@@ -996,9 +1011,9 @@ mod test {
 
         let mut bytecode = bytecode! {
             PUSH32(Word::from(stack.rd_length))
-            PUSH32(Word::from(stack.rd_offset))
+            PUSH32(stack.rd_offset)
             PUSH32(Word::from(stack.cd_length))
-            PUSH32(Word::from(stack.cd_offset))
+            PUSH32(stack.cd_offset)
         };
         if is_call_or_callcode {
             bytecode.push(32, stack.value);

zkevm-circuits/src/evm_circuit/execution/codecopy.rs

@@ -198,7 +198,7 @@ mod tests {
     use eth_types::{bytecode, Word};
     use mock::TestContext;
 
-    fn test_ok(code_offset: Word, memory_offset: usize, size: usize, large: bool) {
+    fn test_ok(code_offset: Word, memory_offset: Word, size: usize, large: bool) {
         let mut code = bytecode! {};
         if large {
             for _ in 0..size {
@@ -208,7 +208,7 @@ mod tests {
         let tail = bytecode! {
             PUSH32(Word::from(size))
             PUSH32(code_offset)
-            PUSH32(Word::from(memory_offset))
+            PUSH32(memory_offset)
             CODECOPY
             STOP
         };
@@ -222,18 +222,23 @@ mod tests {
 
     #[test]
     fn codecopy_gadget_simple() {
-        test_ok(0x00.into(), 0x00, 0x20, false);
-        test_ok(0x30.into(), 0x20, 0x30, false);
-        test_ok(0x20.into(), 0x10, 0x42, false);
+        test_ok(0x00.into(), 0x00.into(), 0x20, false);
+        test_ok(0x30.into(), 0x20.into(), 0x30, false);
+        test_ok(0x20.into(), 0x10.into(), 0x42, false);
     }
 
     #[test]
     fn codecopy_gadget_large() {
-        test_ok(0x102.into(), 0x103, 0x101, true);
+        test_ok(0x102.into(), 0x103.into(), 0x101, true);
     }
 
     #[test]
     fn codecopy_gadget_code_offset_overflow() {
-        test_ok(Word::MAX, 0x103, 0x101, true);
+        test_ok(Word::MAX, 0x103.into(), 0x101, true);
+    }
+
+    #[test]
+    fn codecopy_gadget_overflow_memory_offset_and_zero_size() {
+        test_ok(0x102.into(), Word::MAX, 0, false);
     }
 }

zkevm-circuits/src/evm_circuit/execution/create.rs

@@ -870,4 +870,28 @@ mod test {
             run_test_circuits(test_context(caller));
         }
     }
+
+    #[test]
+    fn test_create_overflow_offset_and_zero_size() {
+        for is_create2 in [true, false] {
+            let mut bytecode = bytecode! {
+                PUSH1(0) // size
+                PUSH32(Word::MAX) // offset
+                PUSH2(23414) // value
+            };
+            bytecode.write_op(if is_create2 {
+                OpcodeId::CREATE2
+            } else {
+                OpcodeId::CREATE
+            });
+            let caller = Account {
+                address: *CALLER_ADDRESS,
+                code: bytecode.into(),
+                nonce: 10.into(),
+                balance: eth(10),
+                ..Default::default()
+            };
+            run_test_circuits(test_context(caller));
+        }
+    }
 }