Open
Conversation
Fix four High severity audit findings related to unchecked arithmetic: 1. Timestamp overflow in decode_block: timestamp_secs * 1000 can overflow for large timestamps. Now uses checked_mul with i64::try_from validation. 2. Log index narrowing in decode_log: log_index and transaction_index were cast with unchecked `as i32`. Now uses i32::try_from() and returns error on overflow. 3. Receipt index narrowing in decode_receipt: transaction_index was cast with unchecked `as i32`. Now uses i32::try_from(). 4. eth_blockNumber u64::MAX overflow in tick_realtime: current_to + 1 and tip_num + 1 could overflow at u64::MAX. Now uses saturating_add. All three decode functions now return Result, propagating errors to callers. Added unit tests for overflow cases and normal value decoding. Amp-Thread-ID: https://ampcode.com/threads/T-019d458d-01b9-76ca-9fb1-b40dd6c6a615 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix four High severity audit findings related to unchecked arithmetic in RPC data decoding.
Changes
Timestamp overflow in
decode_block—timestamp_secs * 1000overflows for large timestamps. Now useschecked_mulwithi64::try_fromvalidation, returning an error instead of silently wrapping.Log index narrowing in
decode_log—log_indexandtransaction_indexwere cast with uncheckedas i32. Now usesi32::try_from()and returns error on overflow.Receipt index narrowing in
decode_receipt—transaction_indexwas cast with uncheckedas i32. Now usesi32::try_from().u64::MAXoverflow intick_realtime—current_to + 1andtip_num + 1could overflow whenremote_headisu64::MAX. Now usessaturating_add.All three decode functions now return
Result, propagating errors to callers throughoutengine.rs.Tests
decode_block_timestamp_overflow_returns_error— verifiesu64::MAXtimestamp returns errordecode_block_normal_timestamp_succeeds— verifies normal timestamps decode correctlydecode_log_index_overflow_returns_error— verifiesu64::MAXlog index returns errordecode_log_tx_index_overflow_returns_error— verifiesu64::MAXtx index returns errordecode_log_normal_values_succeeds— verifies normal log values decode correctlytest_saturating_add_prevents_u64_max_overflow— verifies saturating arithmetic atu64::MAX