Bug: fix to truncate offset from Word to Uint64 when length is zero for some opcodes

bus-mapping/src/circuit_input_builder.rs

@@ -366,10 +366,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

@@ -105,7 +105,9 @@ 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 memory offset to generate copy steps. Since memory
+    // offset could be Uint64 overflow if memory length is zero.
+    let dst_addr = memory_offset.low_u64();
     let src_addr_end = call_data_offset.checked_add(call_data_length).unwrap();
 
     // Reset start offset to end offset if overflow.

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

@@ -31,9 +31,11 @@ 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();
+        // In offset and length are truncated to Uint64 for call opcodes as:
+        // <https://github.com/ethereum/go-ethereum/blob/84c3799e21d61d677965715fe09f8209660b4009/core/vm/instructions.go#L672>
+        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

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

@@ -78,7 +78,9 @@ 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 to generate copy steps. Since offset could be
+    // Uint64 overflow if length is zero.
+    let dst_addr = dst_offset.low_u64();
     let src_addr_end = code_size;
 
     // Reset start offset to end offset if overflow.

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

@@ -18,7 +18,9 @@ impl<const IS_CREATE2: bool> Opcode for DummyCreate<IS_CREATE2> {
         // TODO: replace dummy create here
         let geth_step = &geth_steps[0];
 
-        let offset = geth_step.stack.nth_last(1)?.as_usize();
+        // Get low Uint64 of offset to generate copy steps. Since offset could
+        // be Uint64 overflow if length is zero.
+        let offset = geth_step.stack.nth_last(1)?.low_u64() as usize;
         let length = geth_step.stack.nth_last(2)?.as_usize();
 
         let curr_memory_word_size = (state.call_ctx()?.memory.len() as u64) / 32;

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

@@ -137,7 +137,9 @@ fn gen_copy_event(
     };
     let code_size = bytecode.code.len() as u64;
 
-    let dst_addr = dst_offset.as_u64();
+    // Get low Uint64 of offset to generate copy steps. Since offset could be
+    // Uint64 overflow if length is zero.
+    let dst_addr = dst_offset.low_u64();
     let src_addr_end = code_size;
 
     // Reset start offset to end offset if overflow.

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

@@ -40,7 +40,9 @@ impl Opcode for ReturnRevert {
             call.is_success.to_word(),
         );
 
-        let offset = offset.as_usize();
+        // Get low Uint64 of offset to generate copy steps. Since offset could
+        // be Uint64 overflow if length is zero.
+        let offset = offset.low_u64() as usize;
         let length = length.as_usize();
 
         // Case A in the spec.

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

@@ -123,7 +123,9 @@ 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 destination offset to generate copy steps. Since it
+    // could be Uint64 overflow if length is zero.
+    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,9 @@ impl Opcode for Sha3 {
         let memory = state
             .call_ctx()?
             .memory
-            .read_chunk(offset.as_usize().into(), size.as_usize().into());
+            // Get low Uint64 of offset to generate copy steps. Since offset
+            // could be Uint64 overflow if length is zero.
+            .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 +67,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,

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

@@ -263,9 +263,9 @@ mod test {
 
     fn test_root_ok(
         call_data_length: usize,
-        memory_offset: usize,
         length: usize,
         data_offset: Word,
+        memory_offset: Word,
     ) {
         let bytecode = bytecode! {
             PUSH32(length)
@@ -302,9 +302,9 @@ mod test {
     fn test_internal_ok(
         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]);
 
@@ -343,31 +343,37 @@ mod test {
 
     #[test]
     fn calldatacopy_gadget_simple() {
-        test_root_ok(0x40, 0x40, 10, 0x00.into());
-        test_internal_ok(0x40, 0x40, 0xA0, 10, 0x10.into());
+        test_root_ok(0x40, 10, 0x00.into(), 0x40.into());
+        test_internal_ok(0x40, 0x40, 10, 0x10.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_large() {
-        test_root_ok(0x204, 0x103, 0x101, 0x102.into());
-        test_internal_ok(0x30, 0x204, 0x103, 0x101, 0x102.into());
+        test_root_ok(0x204, 0x101, 0x102.into(), 0x103.into());
+        test_internal_ok(0x30, 0x204, 0x101, 0x102.into(), 0x103.into());
     }
 
     #[test]
     fn calldatacopy_gadget_out_of_bound() {
-        test_root_ok(0x40, 0x40, 40, 0x20.into());
-        test_internal_ok(0x40, 0x20, 0xA0, 10, 0x28.into());
+        test_root_ok(0x40, 40, 0x20.into(), 0x40.into());
+        test_internal_ok(0x40, 0x20, 10, 0x28.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_zero_length() {
-        test_root_ok(0x40, 0x40, 0, 0x00.into());
-        test_internal_ok(0x40, 0x40, 0xA0, 0, 0x10.into());
+        test_root_ok(0x40, 0, 0x00.into(), 0x40.into());
+        test_internal_ok(0x40, 0x40, 0, 0x10.into(), 0xA0.into());
     }
 
     #[test]
     fn calldatacopy_gadget_data_offset_overflow() {
-        test_root_ok(0x40, 0x40, 0, Word::MAX);
-        test_internal_ok(0x40, 0x40, 0xA0, 0, Word::MAX);
+        test_root_ok(0x40, 10, Word::MAX, 0x40.into());
+        test_internal_ok(0x40, 0x40, 10, Word::MAX, 0xA0.into());
+    }
+
+    #[test]
+    fn calldatacopy_gadget_overflow_memory_offset_and_zero_length() {
+        test_root_ok(0x40, 0, 0x40.into(), Word::MAX);
+        test_internal_ok(0x40, 0x40, 0, 0x10.into(), Word::MAX);
     }
 }

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

@@ -691,31 +691,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()
@@ -732,13 +732,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,
     }
 
@@ -765,9 +780,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);
@@ -777,9 +792,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);
@@ -807,9 +822,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

@@ -200,7 +200,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 {
@@ -210,7 +210,7 @@ mod tests {
         let tail = bytecode! {
             PUSH32(Word::from(size))
             PUSH32(code_offset)
-            PUSH32(Word::from(memory_offset))
+            PUSH32(memory_offset)
             CODECOPY
             STOP
         };
@@ -224,18 +224,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);
     }
 }