Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow truncated data in toplevel containers #921

Merged
merged 3 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ jobs:
~/tests/EIPTests/BlockchainTests/
- download_execution_tests:
repo: ipsilon/tests
rev: eof-notxcreate
rev: eof-toplevel
legacy: false
- run:
name: "State tests (EOF)"
Expand Down
12 changes: 10 additions & 2 deletions lib/evmone/eof.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,14 @@ EOFValidationError validate_eof1(evmc_revision rev, bytes_view main_container) n
visited_code_sections.end())
return EOFValidationError::unreachable_code_sections;

if (referenced_by_eofcreate && !header.can_init(container.size()))
return EOFValidationError::eofcreate_with_truncated_container;
// Check if truncated data section is allowed.
if (!header.has_full_data(container.size()))
{
if (main_container == container)
return EOFValidationError::toplevel_container_truncated;
if (referenced_by_eofcreate)
return EOFValidationError::eofcreate_with_truncated_container;
}

// Enqueue subcontainers
for (size_t subcont_idx = 0; subcont_idx < subcontainer_count; ++subcont_idx)
Expand Down Expand Up @@ -927,6 +933,8 @@ std::string_view get_error_message(EOFValidationError err) noexcept
return "invalid_container_section_index";
case EOFValidationError::eofcreate_with_truncated_container:
return "eofcreate_with_truncated_container";
case EOFValidationError::toplevel_container_truncated:
return "toplevel_container_truncated";
case EOFValidationError::impossible:
return "impossible";
}
Expand Down
5 changes: 3 additions & 2 deletions lib/evmone/eof.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ struct EOF1Header
return container.substr(data_offset);
}

/// A helper to check whether the container can be an initcontainer.
[[nodiscard]] bool can_init(size_t container_size) const noexcept
/// A helper to check whether the container has data section body size equal to declare size.
[[nodiscard]] bool has_full_data(size_t container_size) const noexcept
{
// Containers with truncated data section cannot be initcontainers.
const auto truncated_data = static_cast<size_t>(data_offset + data_size) > container_size;
Expand Down Expand Up @@ -138,6 +138,7 @@ enum class EOFValidationError
too_many_container_sections,
invalid_container_section_index,
eofcreate_with_truncated_container,
toplevel_container_truncated,

impossible,
};
Expand Down
4 changes: 0 additions & 4 deletions lib/evmone/instructions_calls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,10 +402,6 @@ Result create_eof_impl(
const auto error_subcont = validate_eof(state.rev, initcontainer);
if (error_subcont != EOFValidationError::success)
return {EVMC_SUCCESS, gas_left}; // "Light" failure.

const auto initcontainer_header = read_valid_eof1_header(initcontainer);
if (!initcontainer_header.can_init(initcontainer.size()))
return {EVMC_SUCCESS, gas_left}; // "Light" failure.
}

evmc_message msg{.kind = to_call_kind(Op)};
Expand Down
2 changes: 1 addition & 1 deletion test/state/host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ std::optional<evmc_message> Host::prepare_message(evmc_message msg) noexcept
if (header == nullptr)
return {}; // Light early exception.

if (!header->can_init(msg.input_size))
if (!header->has_full_data(msg.input_size))
return {}; // Light early exception.

const auto container_size =
Expand Down
2 changes: 2 additions & 0 deletions test/unittests/eof_validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ std::string_view get_tests_error_message(EOFValidationError err) noexcept
return "EOF_InvalidContainerSectionIndex";
case EOFValidationError::eofcreate_with_truncated_container:
return "EOF_EofCreateWithTruncatedContainer";
case EOFValidationError::toplevel_container_truncated:
return "EOF_ToplevelContainerTruncated";
case EOFValidationError::impossible:
return "impossible";
}
Expand Down
21 changes: 16 additions & 5 deletions test/unittests/eof_validation_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,20 @@ TEST_F(eof_validation, EOF1_truncated_section)
EOFValidationError::invalid_section_bodies_size);
add_test_case("EF0001 010004 0200010002 040000 00 00800000 FE",
EOFValidationError::invalid_section_bodies_size);
// Data section may be truncated
add_test_case("EF0001 010004 0200010001 040002 00 00800000 FE", EOFValidationError::success);
add_test_case("EF0001 010004 0200010001 040002 00 00800000 FE AA", EOFValidationError::success);

// Data section may be truncated in runtime subcontainer
add_test_case(
eof_bytecode(returncontract(0, 0, 2), 2).container(eof_bytecode(OP_INVALID).data("", 2)),
EOFValidationError::success);
add_test_case(
eof_bytecode(returncontract(0, 0, 1), 2).container(eof_bytecode(OP_INVALID).data("aa", 2)),
EOFValidationError::success);

// Data section may not be truncated in toplevel container
add_test_case(
eof_bytecode(OP_INVALID).data("", 2), EOFValidationError::toplevel_container_truncated);
add_test_case(
eof_bytecode(OP_INVALID).data("aa", 2), EOFValidationError::toplevel_container_truncated);
}

TEST_F(eof_validation, EOF1_code_section_offset)
Expand Down Expand Up @@ -1016,9 +1027,9 @@ TEST_F(eof_validation, EOF1_embedded_container)
EOFValidationError::success);

// no data section in container, but anticipated aux_data
// data section is allowed to be truncated in runtime subcontainer
add_test_case(
"EF0001 010004 0200010006 0300010014 040002 00 00800001 6000E0000000 "
"EF000101000402000100010400000000800000FE",
eof_bytecode(returncontract(0, 0, 2), 2).container(eof_bytecode(OP_INVALID).data("", 2)),
EOFValidationError::success);

// with data section
Expand Down