Skip to content

Commit

Permalink
Disallow code sections unreachable from section 0 (#866)
Browse files Browse the repository at this point in the history 
- Validates the code by using a worklist of accessed code sections
- At the end if any code section was not accessed will return
unreachable_code_sections error.
- Fixes tests previously containing unreachable code sections
  • Loading branch information
@gumb0
gumb0 authored Apr 26, 2024
2 parents 1b2d6b5 + e8626d2 commit 41cc722
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 15 deletions.
27 changes: 19 additions & 8 deletions lib/evmone/eof.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
#include <limits>
#include <numeric>
#include <ostream>
#include <queue>
#include <span>
#include <stack>
#include <unordered_set>
#include <variant>
#include <vector>

Expand Down Expand Up @@ -251,7 +251,7 @@ std::variant<std::vector<EOFCodeType>, EOFValidationError> validate_types(

EOFValidationError validate_instructions(evmc_revision rev, const EOF1Header& header,
std::span<const EOF1Header> subcontainer_headers, size_t code_idx, bytes_view container,
std::unordered_set<uint16_t>& accessed_code_sections) noexcept
std::queue<uint16_t>& code_sections_worklist) noexcept
{
const bytes_view code{header.get_code(container, code_idx)};
assert(!code.empty()); // guaranteed by EOF headers validation
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -611,7 +611,8 @@ std::variant<EOF1Header, EOFValidationError> validate_eof1( // NOLINT(misc-no-r
}
const auto data_offset = static_cast<uint16_t>(offset);

std::unordered_set<uint16_t> accessed_code_sections = {0};
std::vector<bool> visited_code_sections(code_sizes.size());
std::queue<uint16_t> code_sections_worklist({0});
EOF1Header header{container[2], code_sizes, code_offsets, data_size, data_offset,
container_sizes, container_offsets, types};

Expand All @@ -628,10 +629,19 @@ std::variant<EOF1Header, EOFValidationError> 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;

Expand All @@ -646,7 +656,8 @@ std::variant<EOF1Header, EOFValidationError> 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;
Expand Down
44 changes: 37 additions & 7 deletions test/unittests/eof_validation_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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} +
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}

0 comments on commit 41cc722

Please sign in to comment.