Skip to content

eof: fix multi-container-section case#978

Merged
pdobacz merged 2 commits intomasterfrom
multi-cont-section
Aug 22, 2024
Merged

eof: fix multi-container-section case#978
pdobacz merged 2 commits intomasterfrom
multi-cont-section

Conversation

@pdobacz
Copy link
Copy Markdown
Member

@pdobacz pdobacz commented Aug 21, 2024

Credit to @chfast to find the case via fuzzing, as well as suggest this version of the fix (alternative version in comment, not sure which is cleaner anymore)

@pdobacz pdobacz requested a review from chfast August 21, 2024 13:40
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.29%. Comparing base (85ded61) to head (0855eb1).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #978   +/-   ##
=======================================
  Coverage   94.29%   94.29%           
=======================================
  Files         143      143           
  Lines       16145    16148    +3     
=======================================
+ Hits        15224    15227    +3     
  Misses        921      921           
Flag Coverage Δ
eof_execution_spec_tests 16.67% <66.66%> (+<0.01%) ⬆️
ethereum_tests 26.91% <66.66%> (+<0.01%) ⬆️
ethereum_tests_silkpre 18.67% <0.00%> (-0.01%) ⬇️
execution_spec_tests 17.71% <0.00%> (-0.01%) ⬇️
unittests 89.74% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
lib/evmone/eof.cpp 85.68% <100.00%> (+0.02%) ⬆️
test/unittests/eof_validation_test.cpp 99.36% <100.00%> (+<0.01%) ⬆️

@chfast
Copy link
Copy Markdown
Member

chfast commented Aug 21, 2024

Let me try one more thing here.

@chfast
Copy link
Copy Markdown
Member

chfast commented Aug 21, 2024

I pushed an alternative fix where we check id != expected twice but the helper get_section_missing_error() remains sane.

@chfast chfast added bug Something isn't working EOF labels Aug 21, 2024
@chfast chfast requested a review from gumb0 August 21, 2024 14:47
@pdobacz pdobacz merged commit ce2a7f0 into master Aug 22, 2024
@pdobacz pdobacz deleted the multi-cont-section branch August 22, 2024 11:00
@chfast
Copy link
Copy Markdown
Member

chfast commented Aug 26, 2024

This issue was found indirectly by fuzzing: I was debugging the fuzzer mutation and noticed this weird EOF header. This indicates this validation issue may not be exploitable. Yet, it is a serious bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working EOF

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants