fix(security): Security audit, fix security findings in grevm parallel EVM#99
Merged
AshinGau merged 9 commits intoGalxe:mainfrom Mar 9, 2026
Merged
fix(security): Security audit, fix security findings in grevm parallel EVM#99AshinGau merged 9 commits intoGalxe:mainfrom
AshinGau merged 9 commits intoGalxe:mainfrom
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 <[email protected]>
…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 <[email protected]>
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).
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.
follow #97