eof: Data opcodes zero pad on out of bounds access (#741)

ethereum · Dec 5, 2023 · 754fc9e · 754fc9e
1 parent ecc83d9
commit 754fc9e
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 20 deletions.
diff --git a/lib/evmone/instructions.hpp b/lib/evmone/instructions.hpp
@@ -959,16 +959,23 @@ inline Result mcopy(StackTop stack, int64_t gas_left, ExecutionState& state) noe
     return {EVMC_SUCCESS, gas_left};
 }
 
-inline Result dataload(StackTop stack, int64_t gas_left, ExecutionState& state) noexcept
+inline void dataload(StackTop stack, ExecutionState& state) noexcept
 {
     auto& index = stack.top();
 
-    if (state.data.size() < 32 || (state.data.size() - 32) < index)
-        return {EVMC_INVALID_MEMORY_ACCESS, gas_left};  // TODO: Introduce EVMC_INVALID_DATA_ACCESS
+    if (state.data.size() < index)
+        index = 0;
+    else
+    {
+        const auto begin = static_cast<size_t>(index);
+        const auto end = std::min(begin + 32, state.data.size());
 
-    const auto begin = static_cast<size_t>(index);
-    index = intx::be::unsafe::load<uint256>(&state.data[begin]);
-    return {EVMC_SUCCESS, gas_left};
+        uint8_t data[32] = {};
+        for (size_t i = 0; i < (end - begin); ++i)
+            data[i] = state.data[begin + i];
+
+        index = intx::be::unsafe::load<uint256>(data);
+    }
 }
 
 inline void datasize(StackTop stack, ExecutionState& state) noexcept
@@ -993,20 +1000,21 @@ inline Result datacopy(StackTop stack, int64_t gas_left, ExecutionState& state)
     if (!check_memory(gas_left, state.memory, mem_index, size))
         return {EVMC_OUT_OF_GAS, gas_left};
 
+    const auto dst = static_cast<size_t>(mem_index);
+    // TODO why?
+    const auto src =
+        state.data.size() < data_index ? state.data.size() : static_cast<size_t>(data_index);
     const auto s = static_cast<size_t>(size);
-
-    if (state.data.size() < s || state.data.size() - s < data_index)
-        return {EVMC_INVALID_MEMORY_ACCESS, gas_left};  // TODO: Introduce EVMC_INVALID_DATA_ACCESS
+    const auto copy_size = std::min(s, state.data.size() - src);
 
     if (const auto cost = copy_cost(s); (gas_left -= cost) < 0)
         return {EVMC_OUT_OF_GAS, gas_left};
 
-    if (s > 0)
-    {
-        const auto src = static_cast<size_t>(data_index);
-        const auto dst = static_cast<size_t>(mem_index);
-        std::memcpy(&state.memory[dst], &state.data[src], s);
-    }
+    if (copy_size > 0)
+        std::memcpy(&state.memory[dst], &state.data[src], copy_size);
+
+    if (s - copy_size > 0)
+        std::memset(&state.memory[dst + copy_size], 0, s - copy_size);
 
     return {EVMC_SUCCESS, gas_left};
 }

diff --git a/test/unittests/evm_eof_test.cpp b/test/unittests/evm_eof_test.cpp
@@ -182,15 +182,21 @@ TEST_P(evm, eof1_dataload)
 
     // DATALOAD(33) - truncated word
     execute(code, "0000000000000000000000000000000000000000000000000000000000000021"_hex);
-    EXPECT_STATUS(EVMC_INVALID_MEMORY_ACCESS);
+    EXPECT_STATUS(EVMC_SUCCESS);
+    EXPECT_EQ(bytes_view(result.output_data, result.output_size),
+        "aaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd00"_hex);
 
     // DATALOAD(64) - out of data bounds
     execute(code, "0000000000000000000000000000000000000000000000000000000000000040"_hex);
-    EXPECT_STATUS(EVMC_INVALID_MEMORY_ACCESS);
+    EXPECT_STATUS(EVMC_SUCCESS);
+    EXPECT_EQ(bytes_view(result.output_data, result.output_size),
+        "0000000000000000000000000000000000000000000000000000000000000000"_hex);
 
     // DATALOAD(u256_max) - out of data bounds
     execute(code, "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"_hex);
-    EXPECT_STATUS(EVMC_INVALID_MEMORY_ACCESS);
+    EXPECT_STATUS(EVMC_SUCCESS);
+    EXPECT_EQ(bytes_view(result.output_data, result.output_size),
+        "0000000000000000000000000000000000000000000000000000000000000000"_hex);
 }
 
 TEST_P(evm, eof1_dataloadn)
@@ -319,11 +325,15 @@ TEST_P(evm, eof1_datacopy)
 
     code = eof1_bytecode(bytecode(8) + 63 + 0 + OP_DATACOPY + ret(0, 32), 3, data);
     execute(code);
-    EXPECT_STATUS(EVMC_INVALID_MEMORY_ACCESS);
+    EXPECT_STATUS(EVMC_SUCCESS);
+    EXPECT_EQ(bytes_view(result.output_data, result.output_size),
+        "dd00000000000000000000000000000000000000000000000000000000000000"_hex);
 
     code = eof1_bytecode(bytecode(0) + 65 + 0 + OP_DATACOPY + ret(0, 32), 3, data);
     execute(code);
-    EXPECT_STATUS(EVMC_INVALID_MEMORY_ACCESS);
+    EXPECT_STATUS(EVMC_SUCCESS);
+    EXPECT_EQ(bytes_view(result.output_data, result.output_size),
+        "0000000000000000000000000000000000000000000000000000000000000000"_hex);
 }
 
 TEST_P(evm, datacopy_memory_cost)