Merge pull request #594 from ethereum/analyze-eof-refactor

Enable stack height validation for CALLF

Author: rodiazet

lib/evmone/baseline.cpp

@@ -69,7 +69,7 @@ CodeAnalysis analyze_legacy(bytes_view code)
 
 CodeAnalysis analyze_eof1(bytes_view container)
 {
-    const auto header = read_valid_eof1_header(container);
+    auto header = read_valid_eof1_header(container);
 
     // Extract all code sections as single buffer reference.
     // TODO: It would be much easier if header had code_sections_offset and data_section_offset
@@ -79,17 +79,7 @@ CodeAnalysis analyze_eof1(bytes_view container)
     const auto executable_code =
         container.substr(code_sections_offset, code_sections_end - code_sections_offset);
 
-    // Code section offsets relative to the beginning of the first code section.
-    // Execution starts at position 0 (first code section).
-    // The implementation of CALLF uses these offsets.
-    CodeAnalysis::CodeOffsets relative_offsets;
-    relative_offsets.reserve(header.code_offsets.size());
-    for (const auto offset : header.code_offsets)
-        relative_offsets.push_back(offset - code_sections_offset);
-
-    // FIXME: Better way of getting EOF version.
-    const auto eof_version = container[2];
-    return CodeAnalysis{executable_code, {}, eof_version, relative_offsets};
+    return CodeAnalysis{executable_code, std::move(header)};
 }
 }  // namespace
 
@@ -338,7 +328,7 @@ evmc_result execute(const VM& vm, ExecutionState& state, const CodeAnalysis& ana
 
     const auto code = analysis.executable_code;
 
-    const auto& cost_table = get_baseline_cost_table(state.rev, analysis.eof_version);
+    const auto& cost_table = get_baseline_cost_table(state.rev, analysis.eof_header.version);
 
     auto* tracer = vm.get_tracer();
     if (INTX_UNLIKELY(tracer != nullptr))

lib/evmone/baseline.hpp

@@ -3,6 +3,7 @@
 // SPDX-License-Identifier: Apache-2.0
 #pragma once
 
+#include "eof.hpp"
 #include <evmc/evmc.h>
 #include <evmc/utils.h>
 #include <memory>
@@ -22,14 +23,10 @@ class CodeAnalysis
 {
 public:
     using JumpdestMap = std::vector<bool>;
-    using CodeOffsets = std::vector<uint16_t>;
 
     bytes_view executable_code;  ///< Executable code section.
     JumpdestMap jumpdest_map;    ///< Map of valid jump destinations.
-    uint8_t eof_version = 0;     ///< The EOF version, 0 means legacy code.
-    /// Offset of each code section relative to the beginning of the first code
-    /// section. We flatten the sections for cheap execution.
-    CodeOffsets code_offsets;
+    EOF1Header eof_header;       ///< The EOF header.
 
 private:
     /// Padded code for faster legacy code execution.
@@ -43,11 +40,8 @@ class CodeAnalysis
         m_padded_code{std::move(padded_code)}
     {}
 
-    CodeAnalysis(bytes_view code, JumpdestMap map, uint8_t version, CodeOffsets offsets)
-      : executable_code{code},
-        jumpdest_map{std::move(map)},
-        eof_version{version},
-        code_offsets{std::move(offsets)}
+    CodeAnalysis(bytes_view code, EOF1Header header)
+      : executable_code{code}, eof_header{std::move(header)}
     {}
 };
 static_assert(std::is_move_constructible_v<CodeAnalysis>);

lib/evmone/eof.cpp

@@ -430,7 +430,7 @@ std::variant<EOF1Header, EOFValidationError> validate_eof1(
         offset += code_size;
     }
 
-    EOF1Header header{code_sizes, code_offsets, data_size, types};
+    EOF1Header header{container[2], code_sizes, code_offsets, data_size, types};
 
     for (size_t code_idx = 0; code_idx < header.code_sizes.size(); ++code_idx)
     {
@@ -489,6 +489,8 @@ EOF1Header read_valid_eof1_header(bytes_view container)
 
     EOF1Header header;
 
+    header.version = container[2];
+
     for (auto type_offset = header_size;
          type_offset < header_size + section_headers[TYPE_SECTION][0]; type_offset += 4)
     {

lib/evmone/eof.hpp

@@ -28,6 +28,9 @@ struct EOFCodeType
 
 struct EOF1Header
 {
+    /// The EOF version, 0 means legacy code.
+    uint8_t version = 0;
+
     /// Size of every code section.
     std::vector<uint16_t> code_sizes;
 

lib/evmone/instructions.hpp

@@ -933,11 +933,26 @@ evmc_status_code create_impl(StackTop stack, ExecutionState& state) noexcept;
 inline constexpr auto create = create_impl<OP_CREATE>;
 inline constexpr auto create2 = create_impl<OP_CREATE2>;
 
-inline code_iterator callf(StackTop /*stack*/, ExecutionState& state, code_iterator pos) noexcept
+inline code_iterator callf(StackTop stack, ExecutionState& state, code_iterator pos) noexcept
 {
     const auto index = read_uint16_be(&pos[1]);
+    const auto& header = state.analysis.baseline->eof_header;
+    const auto stack_size = &stack.top() - state.stack_space.bottom();
+    if (stack_size + header.types[index].max_stack_height > StackSpace::limit)
+    {
+        state.status = EVMC_STACK_OVERFLOW;
+        return nullptr;
+    }
+
+    if (state.call_stack.size() >= StackSpace::limit)
+    {
+        // TODO: Add different error code.
+        state.status = EVMC_STACK_OVERFLOW;
+        return nullptr;
+    }
     state.call_stack.push_back(pos + 3);
-    const auto offset = state.analysis.baseline->code_offsets[index];
+
+    const auto offset = header.code_offsets[index] - header.code_offsets[0];
     auto code = state.analysis.baseline->executable_code;
     return code.data() + offset;
 }

test/unittests/evm_eof_function_test.cpp

@@ -57,3 +57,70 @@ TEST_P(evm, eof_function_example2)
     EXPECT_GAS_USED(EVMC_SUCCESS, 44544);
     EXPECT_EQ(output, "0000000000000000000000000000000000000000000000000000000000000262"_hex);
 }
+
+TEST_P(evm, callf_stack_size_1024)
+{
+    // CALLF is not implemented in Advanced.
+    if (is_advanced())
+        return;
+
+    rev = EVMC_CANCUN;
+    const auto code = bytecode{"ef0001 010008 020002 0BFF 0004 030000 00 000003FF 00000001"_hex} +
+                      1023 * push(1) + OP_CALLF + bytecode{"0x0001"_hex} + 1021 * OP_POP +
+                      OP_RETURN + push(1) + OP_POP + OP_RETF;
+
+    ASSERT_EQ(evmone::validate_eof(rev, code), evmone::EOFValidationError::success);
+    execute(bytecode{code});
+    EXPECT_STATUS(EVMC_SUCCESS);
+}
+
+TEST_P(evm, callf_stack_overflow)
+{
+    // CALLF is not implemented in Advanced.
+    if (is_advanced())
+        return;
+
+    rev = EVMC_CANCUN;
+    const auto code =
+        bytecode{"ef0001 010008 020002 0BFF 0007 030000 00 000003FF 00000002"_hex} +  // EOF header
+        1023 * push(1) + OP_CALLF + bytecode{"0x0001"_hex} + 1021 * OP_POP + OP_RETURN +
+        2 * push(1) + 2 * OP_POP + OP_RETF;
+
+    ASSERT_EQ(evmone::validate_eof(rev, code), evmone::EOFValidationError::success);
+    execute(bytecode{code});
+    EXPECT_STATUS(EVMC_STACK_OVERFLOW);
+}
+
+TEST_P(evm, callf_call_stack_size_1024)
+{
+    // CALLF is not implemented in Advanced.
+    if (is_advanced())
+        return;
+
+    rev = EVMC_CANCUN;
+    const auto code = bytecode{"ef0001 010008 020002 0007 000e 030000 00 00000001 01000002"_hex} +
+                      push(1023) + OP_CALLF + bytecode{"0x0001"_hex} + OP_STOP + OP_DUP1 +
+                      OP_RJUMPI + bytecode{"0x0002"_hex} + OP_POP + OP_RETF + push(1) + OP_SWAP1 +
+                      OP_SUB + OP_CALLF + bytecode{"0x0001"_hex} + OP_RETF;
+
+    ASSERT_EQ(evmone::validate_eof(rev, code), evmone::EOFValidationError::success);
+    execute(bytecode{code});
+    EXPECT_STATUS(EVMC_SUCCESS);
+}
+
+TEST_P(evm, callf_call_stack_size_1025)
+{
+    // CALLF is not implemented in Advanced.
+    if (is_advanced())
+        return;
+
+    rev = EVMC_CANCUN;
+    const auto code = bytecode{"ef0001 010008 020002 0007 000e 030000 00 00000001 01000002"_hex} +
+                      push(1024) + OP_CALLF + bytecode{"0x0001"_hex} + OP_STOP + OP_DUP1 +
+                      OP_RJUMPI + bytecode{"0x0002"_hex} + OP_POP + OP_RETF + push(1) + OP_SWAP1 +
+                      OP_SUB + OP_CALLF + bytecode{"0x0001"_hex} + OP_RETF;
+
+    ASSERT_EQ(evmone::validate_eof(rev, code), evmone::EOFValidationError::success);
+    execute(bytecode{code});
+    EXPECT_STATUS(EVMC_STACK_OVERFLOW);
+}