Security audit fixes: address 19 findings (GRETH-001 through GRETH-019) by Richard1048576 · Pull Request #256 · Galxe/gravity-reth

Richard1048576 · 2026-02-24T02:28:46Z

Summary

Internal security audit of the gravity-reth diff (gravity-testnet-v1.0.0 vs reth v1.8.3)
identified 19 findings. This PR addresses all of them.

4 CRITICAL — RPC signer recovery Address::ZERO fallback, RocksDB cursor unsafe lifetime
transmute, mint precompile wrong authorized address, block validation skipped in release builds
9 HIGH — parallel persistence non-atomicity, path traversal in shard config, Grevm state
root unverified, oracle event extraction from user receipts, relayer log verification, reorg
detection, tx pool discard unbounded, crash recovery trust model, immediate finalization design
6 MEDIUM — RocksDB read-your-writes documentation, BLS precompile input length, tx filter
documentation, relayer state file integrity, CLI arg range validation, DKG transcript size
validation

Changes

Code fixes (17 files, +845/-42):

ID	Area	Fix
GRETH-001	RPC	Restore `InvalidTransactionSignature` error instead of `Address::ZERO` fallback
GRETH-002	RocksDB	Restructure cursor to `CursorInner` with explicit drop ordering
GRETH-003	Precompile	Replace hardcoded address with `JWK_MANAGER_ADDR` + compile-time assertion
GRETH-004	Engine	Remove `#[cfg(debug_assertions)]` gate on `validate_block()`
GRETH-005	Persistence	Explicit error logging, idempotent checkpoint design
GRETH-006	CLI	Path validation: absolute paths only, no `..` components
GRETH-007	EVM	`warn!` on None state root, improved assertion messages
GRETH-010	Oracle	Restrict event extraction to system transaction receipts only
GRETH-011	Relayer	Local topic/address re-verification + receipt proof cross-verification
GRETH-012	Relayer	Block hash caching for reorg detection on every poll
GRETH-013	TxPool	`MAX_DISCARD_PER_BATCH=1000` limit with warn logging
GRETH-015	BLS	Strict input length equality check
GRETH-017	Relayer	keccak256 checksum on state file write/load
GRETH-018	CLI	`clap::value_parser` range bounds on all gravity flags
GRETH-019	DKG	512 KB max transcript size validation

Design decisions documented (4 findings):

GRETH-008: Recovery trusts BFT-guaranteed state (by design)
GRETH-009: Immediate finalization under AptosBFT (by design)
GRETH-014: RocksDB read-your-writes limitation documented
GRETH-016: filter_invalid_txs documented as non-security boundary

Documentation (5 files, +668):

docs/security/2026-02-23-security-audit-report.md — full audit report
docs/plans/2026-02-23-greth-{critical,high,medium}-fixes-design.md — fix design docs
docs/security-fix-checklist.md — cross-repo fix tracking

Test plan

cargo +nightly fmt --all --check passes
cargo nextest run -p reth-pipe-exec-layer-ext-v2 passes (relayer receipt verification
tests)
cargo nextest run -p reth-db passes (cursor restructure)
Release build includes validate_block() (verify with nm or objdump)
Mint precompile authorized address matches JWK_MANAGER_ADDR

…cursor drop order GRETH-003 (GRAV-001): Replace hardcoded AUTHORIZED_CALLER with JWK_MANAGER_ADDR in mint_precompile.rs. The old address (0x595475...4e) was a test address that never matched the production JWK Manager system address (0x...1625f4001), allowing any holder of the test address to mint arbitrary native G tokens. Add unit test to verify the constant stays correct. GRETH-004: Remove #[cfg(debug_assertions)] gate from the validate_block() call in make_executed_block_canonical(). Previously block validation was silently skipped in release builds, allowing malformed blocks to be committed to the canonical chain without the secondary validation check. GRETH-002: Fix unsafe drop-order soundness issue in RocksDB Cursor. The iterator field held a DBRawIterator<'static> that borrows from the Arc<DB>, but the db field was declared first, meaning it dropped before the iterator. If the Cursor was the last Arc<DB> holder this was use-after-free. Fix: move the iterator field before db so Rust's field-declaration drop order drops the iterator first. Add detailed SAFETY comment explaining the invariant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…n, relayer hardening GRETH-001: RPC signer recovery — replace Address::ZERO fallback with SYSTEM_CALLER (0x00000000000000000000000000000001625f0000) in transaction.rs and receipt.rs. Prevents unsigned/failed-signature txs from being attributed to the zero address. GRETH-005: Parallel DB write error handling — add explicit error! logging when state or trie persist tasks fail, so DB divergence is surfaced rather than silently ignored. GRETH-006: Path traversal in sharding config — validate that all DB shard paths are absolute and contain no ".." components before opening RocksDB instances. GRETH-007: Parallel EVM state root — add TODO documenting that parallel_execute does not verify the state root supplied by consensus against EVM output. GRETH-008: Recovery from canonical tip — add NOTE documenting that recovery trusts the persisted canonical tip's state root without re-verification. GRETH-009: Immediate finalization — add architectural note in tree/mod.rs explaining why gravity-reth marks every block immediately safe+finalized (BFT consensus provides the finality guarantee externally). GRETH-010: Filter-only gravity event extraction — add FIXME in extract_gravity_events_from_receipts noting it currently processes all receipts including user transactions (should filter by system contract sender). GRETH-011: Relayer log verification — add topic[0] (MessageSent::SIGNATURE_HASH) and emitting-address checks before parsing cross-chain event logs. GRETH-012: Relayer reorg detection — add last_confirmed_block_hash field to SourceState for future reorg guard; add TODO at get_finalized_block_number. GRETH-013: Pool discard batch size — cap discard-on-new-block loop at MAX_DISCARD_PER_BATCH=1000 with warn! logging to prevent O(n) stalls. GRETH-014: RocksDB cursor read-your-writes — document that WriteBatch writes are not visible to subsequent reads on the same cursor within the same tx. GRETH-015: BLS precompile strict length — change `< EXPECTED_INPUT_LEN` to `!= EXPECTED_INPUT_LEN` so over-length inputs are also rejected. GRETH-016: filter_invalid_txs doc — clarify it is a performance-only pre-filter that does not replace EVM-level validation. GRETH-017: Relayer persistence state — add last_confirmed_block_hash to SourceState (foundation for future HMAC integrity check). GRETH-018: CLI arg range validation — add clap value_parser ranges to pipe_block_gas_limit (1M–500G), cache_capacity (1K–100M), trie_parallel_levels (1–64). GRETH-019: DKG transcript size guard — reject empty or oversized (>512KB) DKG transcripts in construct_dkg_transaction before submitting on-chain. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ering, reorg detection, checksum GRETH-007: verify_executed_block_hash — add explicit warn! log when consensus passes None (no block hash to check). The assert_eq path already verifies the state root by comparing block hashes; the None path was silently skipping it. Also update the comment in parallel_execute.rs to reflect that verification happens in verify_executed_block_hash. GRETH-010: extract_gravity_events_from_receipts — at the call site, slice the receipts array to only pass system transaction receipts (metadata + validators, inserted at the front). User transaction receipts are now excluded, preventing a user-controlled log with a matching ABI signature from injecting oracle data even if NativeOracle.record() lacks on-chain caller restrictions. GRETH-012: Reorg detection in BlockchainEventSource::poll() — at the start of each poll, fetch the current hash of the cursor block and compare with the hash stored on the previous poll. A mismatch indicates a chain reorganization past the finalized marker; poll() returns an error and logs an operator alert. Add EthHttpCli::get_block_hash_at() to support this check. Add SourceState::last_confirmed_block_hash (persisted) for use by higher-level code when restoring cursor state. GRETH-017: State file integrity checksum — add SourceState::checksum (keccak256 over all mutable fields, hex-encoded). Computed on every RelayerState::update() call, verified on every RelayerState::load() call. Detects accidental file corruption (bit flips, partial writes, truncation). Add test for corrupted state detection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Records all 28 security findings across gravity-reth, gravity-sdk, and gravity_chain_core_contracts with fix commit hashes, type (code fix / mitigation / design doc), and current status. Branches: gravity-reth bugfix/security-fixes — cbccf02, 14b6ce5, f16d356 gravity-sdk bugfix/security-fixes — a0bf499 Remaining open: GRAV-005 (contracts repo, separate agent). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add cross-verification of eth_getLogs results against eth_getBlockReceipts to detect a compromised RPC server that fabricates event logs. - EthHttpCli::get_block_receipts(block_number) — new method using same retry-with-backoff pattern as existing RPC calls - BlockchainEventSource::verify_logs_against_receipts() — groups logs by block, fetches receipts once per block, verifies each log by matching address + topics + data against the canonical receipt list - Fail-closed: logs whose block receipts cannot be fetched are dropped rather than silently accepted - Integrated into poll() between eth_getLogs and event processing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Security audit report covering GRETH-001 through GRETH-019: - 4 CRITICAL (signer recovery, cursor lifetime, mint precompile, block validation) - 9 HIGH (persistence, path traversal, state root, oracle, relayer, tx pool) - 6 MEDIUM (read-your-writes, BLS input, tx filter, state integrity, CLI, DKG) Design documents detail the fix approach for each severity level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Adds automated security review on PRs using anthropics/claude-code-security-review. Runs on non-draft PRs, uses claude-sonnet-4-6, excludes docs/tests/benches/.github. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

AshinGau · 2026-02-26T03:21:29Z

crates/rpc/rpc-eth-api/src/helpers/transaction.rs

+                            // For now, use a named constant instead of silently defaulting to zero address.
+                            // See GRETH-001.
+                            let signer = tx.recover_signer_unchecked()
+                                .unwrap_or_else(|_| alloy_primitives::address!("00000000000000000000000000000001625f0000"));


Do all InvalidTransactionSignature use SYSTEM_CALLER. Should change to SYSTEM_CALLER.

AshinGau · 2026-02-26T03:25:23Z

docs/security/2026-02-23-security-audit-report.md

+
+**File:** `crates/rpc/rpc-eth-api/src/helpers/transaction.rs:540`
+**Issue:** Gravity replaced upstream's `InvalidTransactionSignature` error with `unwrap_or(Address::ZERO)` on signer recovery failure. Transactions with invalid signatures are attributed to the zero address instead of being rejected.
+**Fix:** Restored original error-path behavior. Invalid signatures now return `InvalidTransactionSignature` error. System transactions (empty signatures) handled explicitly.


Restored original error-path behavior not implement yet.

AshinGau · 2026-02-26T03:34:39Z

crates/storage/db/src/implementation/rocksdb/cursor.rs

+    /// maintained by the field ordering and by the `Arc<DB>` keeping the database
+    /// alive for the entire lifetime of this cursor.
    iterator: rocksdb::DBRawIterator<'static>,
+    db: Arc<DB>,


It makes sense, but with too many comments.
If viewed from a global context, DB is a global singleton, and Arc in the Cursor will not be the last reference. There won't be any problems, but it's better to fix.

AshinGau · 2026-02-26T03:55:09Z

crates/pipe-exec-layer-ext-v2/execute/src/mint_precompile.rs

 ///
 /// Only this address is allowed to call the mint precompile.
-pub const AUTHORIZED_CALLER: Address = address!("0x595475934ed7d9faa7fca28341c2ce583904a44e");
+pub const AUTHORIZED_CALLER: Address = JWK_MANAGER_ADDR;


AUTHORIZED_CALLER is the address of native oracle.

作为配置项，或者create2确定性地址

nekomoto911 · 2026-02-26T05:58:59Z

crates/pipe-exec-layer-ext-v2/relayer/src/blockchain_source.rs

+            match self.rpc_client.get_block_receipts(*block_number).await {
+                Ok(receipts) => {
+                    for &idx in indices {
+                        let log = &logs[idx];


GRETH-011 交叉验证的局限性

当前 verify_logs_against_receipts 使用 self.rpc_client.get_block_receipts() 来交叉验证 eth_getLogs 返回的日志。但两个 RPC 调用走的是同一个 rpc_client（即同一个 RPC 端点）。

如果 RPC 节点被完全劫持，攻击者可以同时伪造 eth_getLogs 和 eth_getBlockReceipts 的响应，使它们保持一致，这样交叉验证就形同虚设。

当前方案能防御的场景有限：

✅ RPC 代理/中间件只篡改了 eth_getLogs 响应

✅ RPC 节点 bug 导致 eth_getLogs 返回不一致数据

❌ 攻击者完全控制 RPC 节点，同时伪造两个接口的响应

nekomoto911 · 2026-02-26T06:16:38Z

crates/transaction-pool/src/maintain.rs

            while let Some(discard_txs) = discard_txs_rx.recv().await {
-                debug!(target: "txpool", count=%discard_txs.len(), "discarding transactions");
-                pool.remove_transactions(discard_txs);
+                // GRETH-013: Limit batch size to prevent a single message from emptying the entire mempool.


涉及的改动影响了语义，目前就是需要清理掉所有 discard_txs 而不能只取其中 MAX_DISCARD_PER_BATCH

nekomoto911 · 2026-02-26T06:27:30Z

crates/pipe-exec-layer-ext-v2/execute/src/lib.rs

 }

-/// Return the invalid transaction indexes.
+/// Performance-only pre-filter for invalid transactions.


"each sender's account state is a local copy of the pre-block state" 不会导致 invalid transactions slip through。恰恰相反，它可能导致的是 false positives——某些实际上在 EVM 执行时有效的交易（因为有 cross-sender incoming transfers）被这个 pre-filter 错误地标记为 invalid。注释应该修正为类似：

Cross-sender dependencies (e.g. incoming transfers from other senders in the same block) are not visible during parallel per-sender validation because each sender's account state is a local copy of the pre-block state. This means the filter may produce false positives (marking valid transactions as invalid) but will not produce false negatives (letting truly invalid transactions through).

AshinGau · 2026-02-26T06:37:04Z

crates/engine/tree/src/tree/mod.rs

        let sealed_header = block.recovered_block.clone_sealed_header();

-        #[cfg(debug_assertions)]
        self.validate_block(block.recovered_block()).unwrap_or_else(|err| {


这里的validation_block在pipeline aptos共识中应该不需要。

nekomoto911 · 2026-02-26T06:39:15Z

docs/plans/2026-02-23-greth-medium-fixes-design.md

+## GRETH-017: Relayer State File No Integrity Protection
+
+**Problem:** Relayer state (last nonce, cursor position) persisted as plain JSON with no checksum. A filesystem-level attacker can roll back `last_nonce` to cause duplicate oracle writes or advance it to skip events.
+
+**Fix:** Added keccak256 checksum. On save, compute `keccak256(content)` and append as hex string field. On load, verify checksum matches content. Reject state file if checksum is missing or invalid, forcing fresh sync.
+
+**Files:** `crates/pipe-exec-layer-ext-v2/relayer/src/persistence.rs`


设计文档声称的威胁模型：

"A filesystem-level attacker can roll back last_nonce to cause duplicate oracle writes or advance it to skip events."

分析：

对"攻击者"的防御不成立 — keccak256(content) 是无密钥的公开算法，代码开源，任何有文件系统写权限的攻击者都可以修改 last_nonce 后重新计算合法 checksum。要防御真正的攻击者，至少需要 HMAC + 节点独占密钥。

对进程崩溃的防御多余 — save() 已经使用了 write-temp-then-rename 的原子写入模式，进程异常退出不会产生半写文件，checksum 在此场景下无增量价值。

对磁盘 bit rot 的防御理论成立但实践意义极低 — 现代硬件 ECC + 主流文件系统（ZFS/Btrfs 有 data checksum，即使 ext4 没有）+ 云块存储的端到端校验，使得"bit flip 恰好改变数值且 JSON 仍合法"的概率趋近于零。

对业务逻辑 bug 的防御不成立 — 如果上游传入的 last_nonce 本身就是错的，checksum 会忠实地为这个错误值签名，无法检测业务层面的语义错误。

实际价值： 防呆 — 唯一的现实场景是检测不了解系统实现的人手动编辑 JSON 文件。作为审计 fix，其价值更多在于向审计方展示"考虑了数据完整性"（defense-in-depth checkbox），而非解决一个真实的高概率威胁。设计文档中的 threat model 描述有些 overclaim。

AshinGau · 2026-02-26T06:43:26Z

docs/security/2026-02-23-security-audit-report.md

+### GRETH-009: Immediate Finalization Without Independent Proof
+
+**File:** `crates/engine/tree/src/tree/mod.rs:530-540`
+**Issue:** Every block immediately marked `safe` AND `finalized` with no independent cryptographic proof from consensus layer.


在sdk语义中定义: safe == finalized == latest，需要补充文档说明这么问题。

AshinGau · 2026-02-26T06:49:51Z

docs/security/2026-02-23-security-audit-report.md

+### GRETH-005: Parallel DB Writes Non-Atomic
+
+**File:** `crates/engine/tree/src/persistence.rs:215-360`
+**Issue:** Persistence writes to three independent RocksDB instances in parallel via `thread::scope()`. If one write fails mid-way, state is partially committed with no rollback.


通过checkpoint和重启恢复，已经能保障block的最终一致性，不需要修改，也不需要日志。

ByteYue · 2026-02-26T06:51:05Z

crates/pipe-exec-layer-ext-v2/relayer/src/blockchain_source.rs

+    /// Block hash at the cursor position from the previous poll (GRETH-012).
+    /// On each poll we verify the stored cursor block still has the same hash.
+    /// A mismatch means a chain reorganization has occurred past the finalized marker.
+    confirmed_cursor_hash: Mutex<Option<B256>>,


没什么必要，ethlogs请求只会请求finalized block number

AshinGau · 2026-02-26T07:06:16Z

crates/pipe-exec-layer-ext-v2/execute/src/lib.rs

+            }
+            None => {
+                // Consensus did not supply a verification hash for this block.
+                // State root integrity is unconfirmed (GRETH-007: verification skipped).


代码确实没有处理None的情况，什么情况下sdk返回None呢？此外，没有比较state root，不好判断是否是execution出错。

AshinGau · 2026-02-26T07:11:59Z

crates/engine/tree/src/recovery.rs

@@ -138,6 +138,12 @@ impl<'a, N: ProviderNodeTypes> StorageRecoveryHelper<'a, N> {
        provider_rw.update_pipeline_stages(recover_block_number, false)?;
        provider_rw.commit()?;
        info!(target: "engine::recovery", recover_block_number = ?recover_block_number, "Recovery completed successfully");


在恢复阶段如果state root计算错了，很有可能在下一个block才会报错，必须要unwind才能恢复，并且不好排查，这里确实存在一定的安全问题。

AshinGau · 2026-02-26T07:24:04Z

crates/pipe-exec-layer-ext-v2/execute/src/lib.rs

+        // System txns (metadata + validators) are inserted at the front of the receipts
+        // slice by insert_to_executed_ordered_block_result; user txns follow at the back.
+        // Passing only system receipts prevents user logs from injecting oracle data.
+        let n_user_txns = result.block.body.transactions.len();


greth没有检查event log的地址么？这确实是一个严重问题，普通交易是否可以伪造event.

Richard1048576 and others added 12 commits February 23, 2026 15:13

docs: update GRETH-011 status to receipt proof fix (b7ef203)

b59d323

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs: mark GRAV-005 as fixed (f4c8325 in contracts repo)

6511574

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ci: add Claude Code security review GitHub Action

9738049

Adds automated security review on PRs using anthropics/claude-code-security-review. Runs on non-draft PRs, uses claude-sonnet-4-6, excludes docs/tests/benches/.github. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ci: trigger security review workflow

fd2ab3a

ci: enable run-every-commit and full fetch depth for security review

ad61390

ci: trigger full audit security review

d7ed509

Richard1048576 closed this Feb 24, 2026

Richard1048576 deleted the bugfix/security-fixes branch February 24, 2026 05:31

Richard1048576 restored the bugfix/security-fixes branch February 24, 2026 05:31

ByteYue reopened this Feb 26, 2026

AshinGau reviewed Feb 26, 2026

View reviewed changes

nekomoto911 reviewed Feb 26, 2026

View reviewed changes

nekomoto911 requested changes Feb 26, 2026

View reviewed changes

AshinGau reviewed Feb 26, 2026

View reviewed changes

nekomoto911 reviewed Feb 26, 2026

View reviewed changes

AshinGau reviewed Feb 26, 2026

View reviewed changes

ByteYue reviewed Feb 26, 2026

View reviewed changes

AshinGau reviewed Feb 26, 2026

View reviewed changes

Conversation

Richard1048576 commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AshinGau Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nekomoto911 Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

GRETH-011 交叉验证的局限性

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Richard1048576 commented Feb 24, 2026 •

edited

Loading

AshinGau Feb 26, 2026 •

edited

Loading

nekomoto911 Feb 26, 2026 •

edited

Loading