Check stack height for OP_CALLF

Co-authored-by: Andrei Maiboroda <[email protected]>
ethereum · Aug 24, 2023 · 450d32d · 450d32d
1 parent 35f4141
commit 450d32d
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 6 deletions.
diff --git a/lib/evmone/eof.cpp b/lib/evmone/eof.cpp
@@ -32,6 +32,7 @@ constexpr auto CODE_SECTION_NUMBER_LIMIT = 1024;
 constexpr auto MAX_STACK_HEIGHT = 0x03FF;
 constexpr auto OUTPUTS_INPUTS_NUMBER_LIMIT = 0x7F;
 constexpr auto REL_OFFSET_SIZE = sizeof(int16_t);
+constexpr auto STACK_SIZE_LIMIT = 1024;
 
 using EOFSectionHeaders = std::array<std::vector<uint16_t>, MAX_SECTION + 1>;
 
@@ -341,18 +342,23 @@ std::variant<EOFValidationError, int32_t> validate_max_stack_height(
         auto stack_height_required = instr::traits[opcode].stack_height_required;
         auto stack_height_change = instr::traits[opcode].stack_height_change;
 
+        auto stack_height = stack_heights[i];
+        assert(stack_height != LOC_UNVISITED);
+
         if (opcode == OP_CALLF)
         {
             const auto fid = read_uint16_be(&code[i + 1]);
 
             stack_height_required = static_cast<int8_t>(code_types[fid].inputs);
+
+            if (stack_height + code_types[fid].max_stack_height - stack_height_required >
+                STACK_SIZE_LIMIT)
+                return EOFValidationError::stack_overflow;
+
             stack_height_change =
                 static_cast<int8_t>(code_types[fid].outputs - stack_height_required);
         }
 
-        auto stack_height = stack_heights[i];
-        assert(stack_height != LOC_UNVISITED);
-
         if (stack_height < stack_height_required)
             return EOFValidationError::stack_underflow;
 
@@ -624,6 +630,8 @@ std::string_view get_error_message(EOFValidationError err) noexcept
         return "unreachable_instructions";
     case EOFValidationError::stack_underflow:
         return "stack_underflow";
+    case EOFValidationError::stack_overflow:
+        return "stack_overflow";
     case EOFValidationError::invalid_code_section_index:
         return "invalid_code_section_index";
     case EOFValidationError::invalid_dataloadn_index:

diff --git a/lib/evmone/eof.hpp b/lib/evmone/eof.hpp
@@ -96,6 +96,7 @@ enum class EOFValidationError
     inputs_outputs_num_above_limit,
     unreachable_instructions,
     stack_underflow,
+    stack_overflow,
     invalid_code_section_index,
     invalid_dataloadn_index,
 

diff --git a/test/unittests/eof_validation_test.cpp b/test/unittests/eof_validation_test.cpp
@@ -24,6 +24,7 @@ TEST(eof_validation, get_error_message)
 {
     EXPECT_EQ(evmone::get_error_message(EOFValidationError::success), "success");
     EXPECT_EQ(evmone::get_error_message(EOFValidationError::invalid_prefix), "invalid_prefix");
+    EXPECT_EQ(evmone::get_error_message(EOFValidationError::stack_overflow), "stack_overflow");
     EXPECT_EQ(evmone::get_error_message(EOFValidationError::impossible), "impossible");
     EXPECT_EQ(evmone::get_error_message(static_cast<EOFValidationError>(-1)), "<unknown>");
 }
@@ -733,6 +734,75 @@ TEST(eof_validation, callf_invalid_code_section_index)
         EOFValidationError::invalid_code_section_index);
 }
 
+TEST(eof_validation, callf_stack_overflow)
+{
+    {
+        auto code =
+            eof1_bytecode(512 * push(1) + OP_CALLF + "0x0000" + 510 * OP_POP + OP_RETURN, 512);
+        EXPECT_EQ(validate_eof(code), EOFValidationError::success);
+    }
+
+    {
+        auto code =
+            eof1_bytecode(513 * push(1) + OP_CALLF + "0x0000" + 511 * OP_POP + OP_RETURN, 513);
+        EXPECT_EQ(validate_eof(code), EOFValidationError::stack_overflow);
+    }
+
+    {
+        auto code =
+            eof1_bytecode(1023 * push(1) + OP_CALLF + "0x0000" + 1021 * OP_POP + OP_RETURN, 1023);
+        EXPECT_EQ(validate_eof(code), EOFValidationError::stack_overflow);
+    }
+}
+
+TEST(eof_validation, callf_with_inputs_stack_overflow)
+{
+    {
+        const auto code =
+            bytecode{"ef0001 010008 020002 0BFD 0003 040000 00 000003FF 02000002"_hex} +
+            1023 * push(1) + OP_CALLF + "0x0001" + 1019 * OP_POP + OP_RETURN + OP_POP + OP_POP +
+            OP_RETF;
+
+        EXPECT_EQ(validate_eof(code), EOFValidationError::success);
+    }
+
+    {
+        const auto code =
+            bytecode{"ef0001 010008 020002 0BFF 0004 040000 00 000003FF 03030004"_hex} +
+            1023 * push(1) + OP_CALLF + "0x0001" + 1021 * OP_POP + OP_RETURN + push(1) + OP_POP +
+            OP_RETF;
+
+        EXPECT_EQ(validate_eof(code), EOFValidationError::success);
+    }
+
+    {
+        const auto code =
+            bytecode{"ef0001 010008 020002 0BFF 0003 040000 00 000003FF 03050005"_hex} +
+            1023 * push(1) + OP_CALLF + "0x0001" + 1021 * OP_POP + OP_RETURN + OP_PUSH0 + OP_PUSH0 +
+            OP_RETF;
+
+        EXPECT_EQ(validate_eof(code), EOFValidationError::stack_overflow);
+    }
+
+    {
+        const auto code =
+            bytecode{"ef0001 010008 020002 0BFF 0005 040000 00 000003FF 03030005"_hex} +
+            1023 * push(1) + OP_CALLF + "0x0001" + 1021 * OP_POP + OP_RETURN + OP_PUSH0 + OP_PUSH0 +
+            OP_POP + OP_POP + OP_RETF;
+
+        EXPECT_EQ(validate_eof(code), EOFValidationError::stack_overflow);
+    }
+
+    {
+        const auto code =
+            bytecode{"ef0001 010008 020002 0C00 0005 040000 00 000003FF 02000003"_hex} +
+            1024 * push(1) + OP_CALLF + "0x0001" + 1020 * OP_POP + OP_RETURN + OP_PUSH0 + OP_POP +
+            OP_POP + OP_POP + OP_RETF;
+
+        EXPECT_EQ(validate_eof(code), EOFValidationError::stack_overflow);
+    }
+}
+
 TEST(eof_validation, incomplete_section_size)
 {
     EXPECT_EQ(
@@ -1302,3 +1372,27 @@ TEST(eof_validation, dataloadn)
                            "00000000000000001111111111111111222222222222222233333333333333"),
         EOFValidationError::invalid_dataloadn_index);
 }
+
+TEST(eof_validation, callf_stack_validation)
+{
+    // function 0: (0, 0) : CALLF{1} STOP
+    // function 1: (0, 1) : PUSH0 PUSH0 CALLF{2} RETF
+    // function 2: (2, 1) : POP RETF
+    EXPECT_EQ(validate_eof("EF0001 01000C 020003000400060002 040000 00 000000010001000202010002 "
+                           "E3000100 5F5FE30002E4 50E4"),
+        EOFValidationError::success);
+
+    // function 0: (0, 0) : CALLF{1} STOP
+    // function 1: (0, 1) : PUSH0 PUSH0 PUSH0 CALLF{2} RETF
+    // function 2: (2, 1) : POP RETF
+    EXPECT_EQ(validate_eof("EF0001 01000C 020003000400070002 040000 00 000000010001000202010002 "
+                           "E3000100 5F5F5FE30002E4 50E4"),
+        EOFValidationError::non_empty_stack_on_terminating_instruction);
+
+    // function 0: (0, 0) : CALLF{1} STOP
+    // function 1: (0, 1) : PUSH0 CALLF{2} RETF
+    // function 2: (2, 1) : POP RETF
+    EXPECT_EQ(validate_eof("EF0001 01000C 020003000400050002 040000 00 000000010001000202010002 "
+                           "E3000100 5FE30002E4 50E4"),
+        EOFValidationError::stack_underflow);
+}
diff --git a/test/unittests/evm_eof_function_test.cpp b/test/unittests/evm_eof_function_test.cpp
@@ -96,9 +96,9 @@ TEST_P(evm, callf_stack_overflow)
 
     rev = EVMC_PRAGUE;
     const auto code =
-        bytecode{"ef0001 010008 020002 0BFF 0007 040000 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;
+        bytecode{"ef0001 01000c 020003 0BFF 0007 0004 040000 00 000003FF 00000001 00000001"_hex} +
+        1023 * push(1) + OP_CALLF + bytecode{"0x0001"_hex} + 1021 * OP_POP + OP_RETURN + push(1) +
+        OP_CALLF + bytecode{"0x0002"_hex} + OP_POP + OP_RETF + push(1) + OP_POP + OP_RETF;
 
     ASSERT_EQ(evmone::validate_eof(rev, code), evmone::EOFValidationError::success);
     execute(bytecode{code});