Security audit: Rounds 1-3 findings in grevm parallel EVM (GREVM-001 ~ R3-009)#97
Draft
Richard1048576 wants to merge 10 commits intomainfrom
Draft
Security audit: Rounds 1-3 findings in grevm parallel EVM (GREVM-001 ~ R3-009)#97Richard1048576 wants to merge 10 commits intomainfrom
Richard1048576 wants to merge 10 commits intomainfrom
Conversation
Internal security audit of grevm parallel EVM engine identified: - 3 CRITICAL: undefined behavior via invalid_reference_casting (7 instances), data race on ContinuousDetectSet, unsound concurrent ParallelState mutation - 3 HIGH: TOCTOU race in async_finality, deadlock risk in TxDependency, next_validation_idx double-increment race - 5 MEDIUM: panic in production paths, env var parsing, hardcoded ERC20 hints, println debug output, fork_join unsound mutation - 5 LOW + 3 INFO: bounds checks, TODOs, memory ordering, redundancy Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…M-019) CRITICAL fixes: - GREVM-001: Replace all 7 instances of `&T → &mut T` UB with proper interior mutability (UnsafeCell, AtomicBool, DisjointVec wrapper) - GREVM-002: Replace Vec<bool> with Vec<AtomicBool> in ContinuousDetectSet - GREVM-003: Wrap ParallelState in UnsafeCell with documented safety invariants for commit thread vs worker thread access HIGH fixes: - GREVM-004: Fix TOCTOU race in async_finality by holding lock across check-and-set - GREVM-005: Fix potential deadlock in TxDependency::add() by enforcing ascending lock order on dependent_state - GREVM-006: Fix double-increment race in next_validation_idx() with compare_exchange loop MEDIUM fixes: - GREVM-007: Replace panic!/assert!/unwrap() in production paths with error returns (async_commit nonce check, balance increment, scheduler commit/validation/execution panics) - GREVM-008: Use unwrap_or(default) for env var parsing - GREVM-009: Fix get_contract_type() to return UNKNOWN for non-ERC20 function selectors (was hardcoded to ERC20) - GREVM-010: Replace println!() with tracing::warn!/debug! - GREVM-011: Introduce DisjointVec<T> wrapper with UnsafeCell for fork-join parallel mutation with documented safety LOW/INFO fixes: - GREVM-014: Upgrade memory ordering to Acquire/Release - GREVM-015: Remove redundant block_size field - GREVM-INFO-001: Remove dead func_id initialization - GREVM-INFO-003: Replace .clone() on Copy types All tests pass (20/20). 3 findings deferred (GREVM-012, 013, 016). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
HIGH findings: - GREVM-R2-001: UnsafeCell safety invariant not enforced by the type system — state_mut() accessible from any thread context - GREVM-R2-002: Commit continues after nonce validation failure — invalid state applied before abort MEDIUM findings: - GREVM-R2-003: fork_join_util produces empty partitions without guard when num_elements < parallel_cnt - GREVM-R2-004: clear_destructed_entry full table scan is O(n) on MVMemory size, potential DoS via SELFDESTRUCT - GREVM-R2-005: tx_dependency.print() clones and locks all state unconditionally under tracing::debug! LOW findings: - GREVM-R2-006: DisjointVec::set has no bounds check - GREVM-R2-007: IdentityHasher::write() panics with unreachable!() - GREVM-R2-008: Unnecessary clone in parallel_apply_transitions via transitions.get().cloned() INFO findings: - GREVM-R2-INFO-001: release profile sets panic="abort" - GREVM-R2-INFO-002: forked dependencies on private branch Filtered out findings matching Round 1 reviewer rejection patterns (algorithmic invariant panics, Block-STM design-level TOCTOU, business-logic-guaranteed lock ordering, consensus-layer bounds).
Richard1048576
changed the title
11 findings (2 HIGH, 4 MEDIUM, 3 LOW, 2 INFO) including TOCTOU re-exploitation in async_finality and CommitGuard alias safety gap. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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
3 participants
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
Multi-round internal security audit of the grevm parallel EVM engine (~3000 LOC).
Round 1 (2026-02-26): 19 findings
invalid_reference_casting(7 instances), data race onContinuousDetectSet::index_flag, unsound concurrent mutation ofParallelStateasync_finality, potential deadlock inTxDependency::add(),next_validation_idx()double-incrementRound 2 (2026-03-02): 10 findings
Round 3 (2026-03-05): 11 findings
async_finalitystill exploitable —lower_ts[txid]never updated byreset_validation_idx(txid+1)new()takes shared refexpect+1u64 overflow in assertionkey_txdependency race with concurrent commitupdate_mv_memoryhides user ETH transfersFix Verification (Round 3)
Cross-Module Findings
assert_eq!= no non-crashing nonce pathArc<Mutex<ParallelState>>incompatible with MVMemorypanic = "abort"bypassesfallback_sequential()entirelyCumulative Statistics
Documentation
docs/security/2026-02-26-security-audit-report.md— Round 1docs/security/2026-03-02-security-audit-round2-report.md— Round 2docs/security-fix-checklist.md— Fix trackingTest plan
cargo +nightly miri test— verify UB fixesRUSTFLAGS="-Z sanitizer=thread" cargo +nightly test— data race detectioncargo bench— no performance regression&mut UnsafeCell)🤖 Generated with Claude Code