diff --git a/lib/evmone/eof.cpp b/lib/evmone/eof.cpp index d6e4dcf17b..0e77eae60a 100644 --- a/lib/evmone/eof.cpp +++ b/lib/evmone/eof.cpp @@ -14,9 +14,9 @@ #include #include #include +#include #include #include -#include #include #include @@ -251,7 +251,7 @@ std::variant, EOFValidationError> validate_types( EOFValidationError validate_instructions(evmc_revision rev, const EOF1Header& header, std::span subcontainer_headers, size_t code_idx, bytes_view container, - std::unordered_set& accessed_code_sections) noexcept + std::queue& code_sections_worklist) noexcept { const bytes_view code{header.get_code(container, code_idx)}; assert(!code.empty()); // guaranteed by EOF headers validation @@ -284,7 +284,7 @@ EOFValidationError validate_instructions(evmc_revision rev, const EOF1Header& he if (header.types[fid].outputs == NON_RETURNING_FUNCTION) return EOFValidationError::callf_to_non_returning_function; if (code_idx != fid) - accessed_code_sections.insert(fid); + code_sections_worklist.push(fid); i += 2; } else if (op == OP_RETF) @@ -301,7 +301,7 @@ EOFValidationError validate_instructions(evmc_revision rev, const EOF1Header& he if (header.types[fid].outputs != NON_RETURNING_FUNCTION) is_returning = true; if (code_idx != fid) - accessed_code_sections.insert(fid); + code_sections_worklist.push(fid); i += 2; } else if (op == OP_DATALOADN) @@ -611,7 +611,8 @@ std::variant validate_eof1( // NOLINT(misc-no-r } const auto data_offset = static_cast(offset); - std::unordered_set accessed_code_sections = {0}; + std::vector visited_code_sections(code_sizes.size()); + std::queue code_sections_worklist({0}); EOF1Header header{container[2], code_sizes, code_offsets, data_size, data_offset, container_sizes, container_offsets, types}; @@ -628,10 +629,19 @@ std::variant validate_eof1( // NOLINT(misc-no-r subcontainer_headers.emplace_back(std::move(subcont_header)); } - for (size_t code_idx = 0; code_idx < header.code_sizes.size(); ++code_idx) + while (!code_sections_worklist.empty()) { + const auto code_idx = code_sections_worklist.front(); + code_sections_worklist.pop(); + + if (visited_code_sections[code_idx]) + continue; + + visited_code_sections[code_idx] = true; + const auto error_instr = validate_instructions( - rev, header, subcontainer_headers, code_idx, container, accessed_code_sections); + rev, header, subcontainer_headers, code_idx, container, code_sections_worklist); + if (error_instr != EOFValidationError::success) return error_instr; @@ -646,7 +656,8 @@ std::variant validate_eof1( // NOLINT(misc-no-r return EOFValidationError::invalid_max_stack_height; } - if (accessed_code_sections.size() != header.code_sizes.size()) + if (std::find(visited_code_sections.begin(), visited_code_sections.end(), false) != + visited_code_sections.end()) return EOFValidationError::unreachable_code_sections; return header; diff --git a/test/unittests/eof_validation_test.cpp b/test/unittests/eof_validation_test.cpp index 4b1ae7aba7..316132acd5 100644 --- a/test/unittests/eof_validation_test.cpp +++ b/test/unittests/eof_validation_test.cpp @@ -691,8 +691,8 @@ TEST_F(eof_validation, max_stack_height) 0x400 * OP_POP + OP_STOP + OP_RETF, EOFValidationError::max_stack_height_above_limit); - add_test_case(bytecode{"EF0001 010008 02000200010C01 040000 00 00800000 000003FF"} + OP_STOP + - 0x400 * bytecode{1} + 0x400 * OP_POP + OP_RETF, + add_test_case(eof_bytecode(callf(1) + OP_STOP, 0) + .code(0x400 * bytecode{1} + 0x400 * OP_POP + OP_RETF, 0, 0, 1023), EOFValidationError::invalid_max_stack_height); add_test_case("EF0001 010008 0200020C010001 040000 00 008003FF 00000000" + 0x400 * bytecode{1} + @@ -858,23 +858,24 @@ TEST_F(eof_validation, non_returning_status) EOFValidationError::success); // Invalid with RETF - add_test_case("EF0001 010008 02000200010001 040000 00 0080000000800000 00 E4", + add_test_case(eof_bytecode(jumpf(1)).code(OP_RETF, 0, 0x80, 0), EOFValidationError::invalid_non_returning_flag); add_test_case("EF0001 010004 0200010001 040000 00 00800000 E4", EOFValidationError::invalid_non_returning_flag); // Invalid with JUMPF to returning add_test_case( - "EF0001 01000c 020003000100030001 040000 00 008000000080000000000000 00 E50002 E4", + "EF0001 01000c 020003000300030001 040000 00 008000000080000000000000 E50001 E50002 E4", EOFValidationError::invalid_non_returning_flag); // Invalid with JUMPF to non-returning - add_test_case("EF0001 010008 02000200010003 040000 00 0080000000000000 00 E50000", + add_test_case("EF0001 010008 02000200030003 040000 00 0080000000000000 E50001 E50000", EOFValidationError::invalid_non_returning_flag); // Invalid with JUMPF to returning and RETF add_test_case( - "EF0001 01000C 020003000100070001 040000 00 008000000180000100000000 00 E10001E4E50002 E4", + "EF0001 01000C 020003000400070001 040000 00 008000010180000100000000 5FE50001 " + "E10001E4E50002 E4", EOFValidationError::invalid_non_returning_flag); // Invalid with JUMPF to non-returning and RETF - add_test_case("EF0001 010008 02000200010007 040000 00 0080000001800001 00 E10001E4E50000", + add_test_case("EF0001 010008 02000200040007 040000 00 0080000101800001 5FE50001 E10001E4E50000", EOFValidationError::invalid_non_returning_flag); // Circular JUMPF: can be both returning and non-returning @@ -971,6 +972,20 @@ TEST_F(eof_validation, unreachable_code_sections) // Code Section 254 calls itself instead of code section 255, leaving code section 255 // unreachable add_test_case(code_sections_256_err_254, EOFValidationError::unreachable_code_sections); + + // Code Section 0 calls section 1, which calls itself, leaving section + // 2 unreachable + add_test_case(eof_bytecode(jumpf(1)).code(jumpf(1), 0, 0x80, 0).code(jumpf(2), 0, 0x80, 0), + EOFValidationError::unreachable_code_sections); + + // Code Section 0 calls section 1, which calls section 2, section 3 and + // 4 call each other but are not reachable from section 0 + add_test_case(eof_bytecode(jumpf(1)) + .code(jumpf(2), 0, 0x80, 0) + .code(OP_INVALID, 0, 0x80, 0) + .code(jumpf(4), 0, 0x80, 0) + .code(jumpf(3), 0, 0x80, 0), + EOFValidationError::unreachable_code_sections); } } @@ -1166,3 +1181,18 @@ TEST_F(eof_validation, EOF1_unreferenced_subcontainer_valid) const auto embedded = eof_bytecode(bytecode{OP_INVALID}); add_test_case(eof_bytecode(OP_STOP).container(embedded), EOFValidationError::success); } + +TEST_F(eof_validation, EOF1_subcontainer_containing_unreachable_code_sections) +{ + const auto embedded_1 = eof_bytecode(OP_INVALID).code(OP_INVALID, 0, 0x80, 0); + add_test_case(eof_bytecode(OP_INVALID).container(embedded_1), + EOFValidationError::unreachable_code_sections); + + const auto embedded_2 = eof_bytecode(jumpf(1)) + .code(jumpf(2), 0, 0x80, 0) + .code(OP_INVALID, 0, 0x80, 0) + .code(jumpf(4), 0, 0x80, 0) + .code(jumpf(3), 0, 0x80, 0); + add_test_case(eof_bytecode(OP_INVALID).container(embedded_2), + EOFValidationError::unreachable_code_sections); +}