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

EOF validation of subcontainer kinds #876

Merged
merged 8 commits into from
Jun 24, 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 @@ -457,7 +457,7 @@ jobs:
~/tests/EIPTests/BlockchainTests/
- download_execution_tests:
repo: ipsilon/tests
rev: eof-toplevel
rev: eof-initcode-tests
legacy: false
- run:
name: "State tests (EOF)"
Expand Down
4 changes: 3 additions & 1 deletion lib/evmone/baseline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,9 @@ evmc_result execute(evmc_vm* c_vm, const evmc_host_interface* host, evmc_host_co

if (vm->validate_eof && rev >= EVMC_PRAGUE && is_eof_container(container))
{
if (validate_eof(rev, container) != EOFValidationError::success)
const auto container_kind =
(msg->depth == 0 ? ContainerKind::initcode : ContainerKind::runtime);
if (validate_eof(rev, container_kind, container) != EOFValidationError::success)
return evmc_make_result(EVMC_CONTRACT_VALIDATION_FAILURE, 0, 0, nullptr, 0);
}

Expand Down
56 changes: 46 additions & 10 deletions lib/evmone/eof.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@
};

std::variant<InstructionValidationResult, EOFValidationError> validate_instructions(
evmc_revision rev, const EOF1Header& header, size_t code_idx, bytes_view container) noexcept
evmc_revision rev, const EOF1Header& header, ContainerKind kind, size_t code_idx,
bytes_view container) noexcept
{
const bytes_view code{header.get_code(container, code_idx)};
assert(!code.empty()); // guaranteed by EOF headers validation
Expand Down Expand Up @@ -323,9 +324,20 @@
if (container_idx >= header.container_sizes.size())
return EOFValidationError::invalid_container_section_index;

if (op == OP_RETURNCONTRACT)
{
if (kind == ContainerKind::runtime || kind == ContainerKind::initcode_runtime)
return EOFValidationError::incompatible_container_kind;
}

subcontainer_references.emplace_back(container_idx, Opcode{op});
++i;
}
else if (op == OP_RETURN || op == OP_STOP)
{
if (kind == ContainerKind::initcode || kind == ContainerKind::initcode_runtime)
return EOFValidationError::incompatible_container_kind;
}
else
i += instr::traits[op].immediate_size;
}
Expand Down Expand Up @@ -572,20 +584,22 @@
return max_stack_height_it->max;
}

EOFValidationError validate_eof1(evmc_revision rev, bytes_view main_container) noexcept
EOFValidationError validate_eof1(
evmc_revision rev, ContainerKind main_container_kind, bytes_view main_container) noexcept
{
struct ContainerValidation
{
bytes_view bytes;
bool referenced_by_eofcreate = false;
ContainerKind kind;
};
// Queue of containers left to process
std::queue<ContainerValidation> container_queue;
container_queue.push({main_container, false});

container_queue.push({main_container, main_container_kind});

while (!container_queue.empty())
{
const auto& [container, referenced_by_eofcreate] = container_queue.front();
const auto& [container, container_kind] = container_queue.front();

// Validate header
auto error_or_header = validate_header(rev, container);
Expand All @@ -603,6 +617,7 @@

const auto subcontainer_count = header.container_sizes.size();
std::vector<bool> subcontainer_referenced_by_eofcreate(subcontainer_count, false);
std::vector<bool> subcontainer_referenced_by_returncontract(subcontainer_count, false);

while (!code_sections_queue.empty())
{
Expand All @@ -616,7 +631,7 @@

// Validate instructions
const auto instr_validation_result_or_error =
validate_instructions(rev, header, code_idx, container);
validate_instructions(rev, header, container_kind, code_idx, container);
if (const auto* error =
std::get_if<EOFValidationError>(&instr_validation_result_or_error))
return *error;
Expand All @@ -629,6 +644,10 @@
{
if (opcode == OP_EOFCREATE)
subcontainer_referenced_by_eofcreate[index] = true;
else if (opcode == OP_RETURNCONTRACT)
subcontainer_referenced_by_returncontract[index] = true;
else
intx::unreachable();

Check warning on line 650 in lib/evmone/eof.cpp

View check run for this annotation

Codecov / codecov/patch

lib/evmone/eof.cpp#L650

Added line #L650 was not covered by tests
}

// TODO(C++23): can use push_range()
Expand Down Expand Up @@ -657,7 +676,8 @@
{
if (main_container == container)
return EOFValidationError::toplevel_container_truncated;
if (referenced_by_eofcreate)
if (container_kind == ContainerKind::initcode ||
container_kind == ContainerKind::initcode_runtime)
return EOFValidationError::eofcreate_with_truncated_container;
}

Expand All @@ -666,7 +686,18 @@
{
const bytes_view subcontainer{header.get_container(container, subcont_idx)};

container_queue.push({subcontainer, subcontainer_referenced_by_eofcreate[subcont_idx]});
const bool eofcreate = subcontainer_referenced_by_eofcreate[subcont_idx];
const bool returncontract = subcontainer_referenced_by_returncontract[subcont_idx];

// TODO Validate whether subcontainer was referenced by any instruction

auto subcontainer_kind = ContainerKind::initcode_runtime;
if (!eofcreate)
subcontainer_kind = ContainerKind::runtime;
else if (!returncontract)
subcontainer_kind = ContainerKind::initcode;

container_queue.push({subcontainer, subcontainer_kind});
}

container_queue.pop();
Expand Down Expand Up @@ -852,9 +883,10 @@
0;
}

EOFValidationError validate_eof(evmc_revision rev, bytes_view container) noexcept
EOFValidationError validate_eof(
evmc_revision rev, ContainerKind kind, bytes_view container) noexcept
{
return validate_eof1(rev, container);
return validate_eof1(rev, kind, container);
}

std::string_view get_error_message(EOFValidationError err) noexcept
Expand Down Expand Up @@ -935,6 +967,10 @@
return "eofcreate_with_truncated_container";
case EOFValidationError::toplevel_container_truncated:
return "toplevel_container_truncated";
case EOFValidationError::ambiguous_container_kind:
return "ambiguous_container_kind";
case EOFValidationError::incompatible_container_kind:
return "incompatible_container_kind";

Check warning on line 973 in lib/evmone/eof.cpp

View check run for this annotation

Codecov / codecov/patch

lib/evmone/eof.cpp#L970-L973

Added lines #L970 - L973 were not covered by tests
case EOFValidationError::impossible:
return "impossible";
}
Expand Down
15 changes: 14 additions & 1 deletion lib/evmone/eof.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,23 @@ enum class EOFValidationError
invalid_container_section_index,
eofcreate_with_truncated_container,
toplevel_container_truncated,
ambiguous_container_kind,
incompatible_container_kind,

impossible,
};

enum class ContainerKind : uint8_t
{
/// Container that uses RETURNCONTRACT. Can be used by EOFCREATE/TXCREATE/Creation transaction.
initcode,
/// Container that uses STOP/RETURN. Can be returned by RETURNCONTRACT.
runtime,
/// Container that uses only REVERT/INVALID or does not terminate execution.
/// Can be used in any context.
initcode_runtime,
};

/// Determines the EOF version of the container by inspecting container's EOF prefix.
/// If the prefix is missing or invalid, 0 is returned meaning legacy code.
[[nodiscard]] uint8_t get_eof_version(bytes_view container) noexcept;
Expand All @@ -153,7 +166,7 @@ enum class EOFValidationError

/// Validates whether given container is a valid EOF according to the rules of given revision.
[[nodiscard]] EVMC_EXPORT EOFValidationError validate_eof(
evmc_revision rev, bytes_view container) noexcept;
evmc_revision rev, ContainerKind kind, bytes_view container) noexcept;

/// Returns the error message corresponding to an error code.
[[nodiscard]] EVMC_EXPORT std::string_view get_error_message(EOFValidationError err) noexcept;
Expand Down
20 changes: 2 additions & 18 deletions lib/evmone/instructions.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,9 @@ inline constexpr auto pop = noop;
inline constexpr auto jumpdest = noop;

template <evmc_status_code Status>
inline TermResult stop_impl(StackTop /*stack*/, int64_t gas_left, ExecutionState& state) noexcept
inline TermResult stop_impl(
StackTop /*stack*/, int64_t gas_left, ExecutionState& /*state*/) noexcept
{
// STOP is forbidden inside EOFCREATE context
if constexpr (Status == EVMC_SUCCESS)
{
if (state.msg->kind == EVMC_EOFCREATE)
return {EVMC_UNDEFINED_INSTRUCTION, gas_left};
}

return {Status, gas_left};
}
inline constexpr auto stop = stop_impl<EVMC_SUCCESS>;
Expand Down Expand Up @@ -1160,13 +1154,6 @@ inline code_iterator jumpf(StackTop stack, ExecutionState& state, code_iterator
template <evmc_status_code StatusCode>
inline TermResult return_impl(StackTop stack, int64_t gas_left, ExecutionState& state) noexcept
{
// RETURN is forbidden inside EOFCREATE context
if constexpr (StatusCode == EVMC_SUCCESS)
{
if (state.msg->kind == EVMC_EOFCREATE)
return {EVMC_UNDEFINED_INSTRUCTION, gas_left};
}

const auto& offset = stack[0];
const auto& size = stack[1];

Expand All @@ -1187,9 +1174,6 @@ inline TermResult returncontract(
const auto& offset = stack[0];
const auto& size = stack[1];

if (state.msg->kind != EVMC_EOFCREATE)
return {EVMC_UNDEFINED_INSTRUCTION, gas_left};

if (!check_memory(gas_left, state.memory, offset, size))
return {EVMC_OUT_OF_GAS, gas_left};

Expand Down
2 changes: 1 addition & 1 deletion lib/evmone/instructions_calls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ Result create_eof_impl(

if constexpr (Op == OP_TXCREATE)
{
const auto error_subcont = validate_eof(state.rev, initcontainer);
const auto error_subcont = validate_eof(state.rev, ContainerKind::initcode, initcontainer);
if (error_subcont != EOFValidationError::success)
return {EVMC_SUCCESS, gas_left}; // "Light" failure.
}
Expand Down
2 changes: 1 addition & 1 deletion test/eofparse/eofparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ int main()
}

const auto& eof = *o;
const auto err = evmone::validate_eof(EVMC_PRAGUE, eof);
const auto err = evmone::validate_eof(EVMC_PRAGUE, evmone::ContainerKind::runtime, eof);
if (err != evmone::EOFValidationError::success)
{
std::cout << "err: " << evmone::get_error_message(err) << "\n";
Expand Down
3 changes: 2 additions & 1 deletion test/eofparsefuzz/eofparsefuzz.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t data_size) noexcept
{
const evmone::bytes_view eof{data, data_size};
if (evmone::validate_eof(EVMC_PRAGUE, eof) == evmone::EOFValidationError::success)
if (evmone::validate_eof(EVMC_PRAGUE, evmone::ContainerKind::runtime, eof) ==
evmone::EOFValidationError::success)
(void)evmone::read_valid_eof1_header(eof);
return 0;
}
22 changes: 19 additions & 3 deletions test/eoftest/eoftest_runner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,39 @@
{
struct Expectation
{
evmc_revision rev;
bool result;
evmc_revision rev = EVMC_PRAGUE;
bool result = false;
};
std::string name;
evmc::bytes code;
ContainerKind kind = ContainerKind::runtime;
std::vector<Expectation> expectations;
};
std::unordered_map<std::string, Case> cases;
};

ContainerKind container_kind_from_string(std::string_view s)
{
if (s == "runtime")
return ContainerKind::runtime;
else if (s == "initcode")
return ContainerKind::initcode;
else if (s == "initcode_runtime")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it makes much sense to support this mode in the tests. If we don't then probably validate_eof() also shouldn't support it as input (should fail with special error?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is proposed to remove this from the spec and #916 has a commit that implements the removal.

return ContainerKind::initcode_runtime;
else
throw std::invalid_argument{"unknown container kind"};

Check warning on line 45 in test/eoftest/eoftest_runner.cpp

View check run for this annotation

Codecov / codecov/patch

test/eoftest/eoftest_runner.cpp#L45

Added line #L45 was not covered by tests
}

void from_json(const json::json& j, EOFValidationTest::Case& o)
{
const auto op_code = evmc::from_hex(j.at("code").get<std::string>());
if (!op_code)
throw std::invalid_argument{"code is invalid hex string"};
o.code = *op_code;

if (const auto it_kind = j.find("kind"); it_kind != j.end())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this standardized?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not supported anywhere, but I plan to try to add this to EEST.

o.kind = container_kind_from_string(it_kind->get<std::string>());

for (const auto& [rev, result] : j.at("results").items())
{
o.expectations.push_back({to_rev(rev), result.at("result").get<bool>()});
Expand Down Expand Up @@ -67,7 +83,7 @@
{
for (const auto& expectation : cases.expectations)
{
const auto result = evmone::validate_eof(expectation.rev, cases.code);
const auto result = evmone::validate_eof(expectation.rev, cases.kind, cases.code);
const bool b_result = (result == EOFValidationError::success);
EXPECT_EQ(b_result, expectation.result)
<< name << " " << expectation.rev << " " << hex(cases.code);
Expand Down
5 changes: 3 additions & 2 deletions test/state/host.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ std::optional<evmc_message> Host::prepare_message(evmc_message msg) noexcept
msg.input_data = msg.input_data + container_size;
msg.input_size = msg.input_size - container_size;

if (validate_eof(m_rev, {msg.code, msg.code_size}) !=
if (validate_eof(m_rev, ContainerKind::initcode, {msg.code, msg.code_size}) !=
EOFValidationError::success)
return {}; // Light early exception.

Expand Down Expand Up @@ -367,7 +367,8 @@ evmc::Result Host::create(const evmc_message& msg) noexcept
// It must be valid EOF, which was validated before execution.
if (msg.kind != EVMC_EOFCREATE)
return evmc::Result{EVMC_CONTRACT_VALIDATION_FAILURE};
assert(validate_eof(m_rev, code) == EOFValidationError::success);
assert(
validate_eof(m_rev, ContainerKind::runtime, code) == EOFValidationError::success);
}
else if (m_rev >= EVMC_LONDON)
{
Expand Down
2 changes: 1 addition & 1 deletion test/statetest/statetest_loader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ void validate_state(const TestState& state, evmc_revision rev)
{
if (rev >= EVMC_PRAGUE)
{
if (const auto result = validate_eof(rev, acc.code);
if (const auto result = validate_eof(rev, ContainerKind::runtime, acc.code);
result != EOFValidationError::success)
{
throw std::invalid_argument(
Expand Down
4 changes: 3 additions & 1 deletion test/unittests/eof_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ TEST(eof, read_valid_eof1_header)
for (const auto& test_case : test_cases)
{
const auto code = from_spaced_hex(test_case.code).value();
EXPECT_EQ(validate_eof(EVMC_PRAGUE, code), EOFValidationError::success) << test_case.code;
EXPECT_EQ(
validate_eof(EVMC_PRAGUE, ContainerKind::runtime, code), EOFValidationError::success)
<< test_case.code;

const auto header = read_valid_eof1_header(code);
EXPECT_EQ(header.code_sizes, test_case.code_sizes) << test_case.code;
Expand Down
Loading