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

precompiles: Improve output buffer handling #951

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

chfast
Copy link
Member

@chfast chfast commented Jul 11, 2024

Replace the fixed-size on-stack temporary buffer for precompiles with a heap-allocated buffer big enough to handle properly any precompile invocation.

This actually keeps the number of allocations the same. Previously the contents of the on-stack buffer were copied to heap by the Result constructor. Now we are creating the heap buffer in the first place and pass the ownership of it to the Result.

This fixes out-of-bounds memory accesses often being found by fuzzers.

Copy link

codecov bot commented Jul 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.29%. Comparing base (70ca837) to head (e186095).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #951   +/-   ##
=======================================
  Coverage   94.29%   94.29%           
=======================================
  Files         143      143           
  Lines       16119    16120    +1     
=======================================
+ Hits        15199    15200    +1     
  Misses        920      920           
Flag Coverage Δ
eof_execution_spec_tests 16.65% <0.00%> (-0.01%) ⬇️
ethereum_tests 26.99% <100.00%> (+<0.01%) ⬆️
ethereum_tests_silkpre 18.71% <100.00%> (+<0.01%) ⬆️
execution_spec_tests 17.98% <100.00%> (+<0.01%) ⬆️
unittests 89.66% <0.00%> (-0.01%) ⬇️

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

Files Coverage Δ
test/state/precompiles.cpp 100.00% <100.00%> (ø)

@chfast chfast force-pushed the precompiles/output_buf branch from c5c79e6 to 156fc7f Compare July 11, 2024 21:26
@chfast chfast requested a review from gumb0 July 11, 2024 23:52
test/state/precompiles.cpp Outdated Show resolved Hide resolved
Replace the fixed-size on-stack temporary buffer for precompiles
with a heap-allocated buffer big enough to handle properly any
precompile invocation.

This actually keeps the number of allocations the same. Previously
the contents of the on-stack buffer were copied to heap
by the Result constructor. Now we are creating the heap buffer
in the first place and pass the ownership of it to the Result.

This fixes out-of-bounds memory accesses often being found by fuzzers.
@chfast chfast force-pushed the precompiles/output_buf branch from 156fc7f to e186095 Compare July 14, 2024 08:05
@chfast chfast merged commit b7bee06 into master Jul 16, 2024
26 checks passed
@chfast chfast deleted the precompiles/output_buf branch July 16, 2024 14:41
@chfast chfast mentioned this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants