security: P0/P1 audit fixes (C-1, C-3, H-1, H-3, H-4)#50
Conversation
C-1: charge batch precompile gas before verification loop; cap count at 256 C-3: verify vote signatures in handle_wpoa_vote (mirror view_change path) H-1: gate L2 recursive-proof stub behind stub-l2-verifier feature H-3: unify batch precompile algorithm dispatch with single verify H-4: document hickory-proto advisory acceptance with full mitigation rationale C-1 (crates/evm/src/precompiles.rs): Add MAX_BATCH_SIGNATURES=256 constant; parse count from header before any work; reject oversized batches upfront; charge gas via checked_mul BEFORE entering the verification loop. C-3 (crates/node/src/node/p2p_handlers.rs): In handle_wpoa_vote, after the FF.6 finality check, look up the voter's public key from known_authorities, infer sig algorithm via infer_signature_type_from_address, verify the signature against block_hash.as_bytes() using MultiVerifier, and drop+penalise on failure — exactly mirroring handle_wpoa_view_change. Update existing wpoa_handle_vote_reaches_quorum test to use real key pairs and valid sigs. H-1 (crates/node/src/node/system_rewards.rs, crates/node/Cargo.toml, crates/node/src/node/mod.rs): Replace soft-pass on RecursiveProof decode error and NotImplemented with hard NodeError::Startup unless compiled with the new stub-l2-verifier feature (default=off). Add startup warn! banner in Node::new when feature is enabled. H-3 (crates/evm/src/precompiles.rs): Replace verify_legacy_mldsa65_item (Dilithium3-only) in batch loop with verify_mldsa65 (ML-DSA-65 first, then Dilithium3 fallback) to match the single-verify dispatch path. H-4 (deny.toml): Expand the hickory-proto advisory acceptance comment with full mitigation rationale (DNS-only bootstrap usage, failure mode analysis). cargo update -p hickory-proto produced no change — no compatible patched release available for libp2p 0.56. Security audit ref: 2026-05-24 consolidated report Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR applies a set of security-audit remediations across the EVM precompiles, node consensus message handling, and STARK/L2 settlement verification behavior, plus a clarified advisory acceptance rationale.
Changes:
- Harden ML-DSA batch precompile against DoS by capping batch count and charging gas before any verification work.
- Require WPoA vote signatures to be verified before dispatch into the consensus round.
- Make STARK L2 recursive proof verification failures hard errors by default, with an explicitly opt-in
stub-l2-verifierfeature for non-production environments.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
deny.toml |
Expands audit advisory ignore rationale and documents mitigation assumptions for the accepted RUSTSEC items. |
crates/node/src/node/system_rewards.rs |
Converts prior “soft-pass” STARK recursive verifier paths into hard errors unless stub-l2-verifier is enabled. |
crates/node/src/node/p2p_handlers.rs |
Adds PQ signature verification for incoming WPoA votes before consensus processing. |
crates/node/src/node/mod.rs |
Emits a prominent startup warning when stub-l2-verifier is enabled; gates related tests by feature. |
crates/node/Cargo.toml |
Introduces the stub-l2-verifier feature (default off). |
crates/evm/src/precompiles.rs |
Adds batch-size cap and pre-charged gas for ML-DSA batch verify; unifies batch algorithm dispatch with single-verify; adds targeted tests. |
Cargo.lock |
Updates locked dependency versions as part of the overall security maintenance work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let item_start = cursor - 4; // include pk_len prefix | ||
| let item = &input[item_start..]; | ||
| valid &= verify_legacy_mldsa65_item(item); | ||
| valid &= verify_mldsa65(item); | ||
| // Advance cursor by pk_len + 4 (msg_len) + msg_len + sig_len |
There was a problem hiding this comment.
Fixed in a273ca7: per-item slice now ends at exact item boundary (item_start..item_end where item_end = cursor + pk_len + 4 + msg_len + DILITHIUM3_SIGNATURE_BYTES) computed before calling verify_mldsa65, so multi-item batches no longer see trailing bytes from subsequent items. Regression tests added: batch_verify_multi_item_happy_path (count=2 both valid) + batch_verify_multi_item_tampered_sig_fails (count=2, second sig tampered → returns false).
| let typed_sig = shell_crypto::PQSignature::new(sig_type, sig.data.clone()); | ||
| let verifier = MultiVerifier; | ||
| match verifier.verify(&pubkey, block_hash.as_bytes(), &typed_sig) { | ||
| Ok(true) => {} // valid — proceed to consensus |
There was a problem hiding this comment.
Fixed in a273ca7: handle_wpoa_vote now rejects (with warn log + peer penalty) any vote where sig.sig_type != infer_signature_type_from_address(voter), eliminating the algorithm-tag confusion. The check fires before typed_sig is constructed and before the vote is dispatched to consensus, so mismatched-tag votes can never reach the commit certificate. Unit test wpoa_handle_vote_rejects_sig_type_mismatch asserts the round stays in Voting when a vote arrives with a valid Dilithium3 signature but sig_type set to MlDsa65.
| /// C-1: count > MAX_BATCH_SIGNATURES must be rejected immediately. | ||
| #[test] | ||
| fn batch_verify_rejects_oversized_count() { | ||
| let mut input = Vec::new(); | ||
| input.extend_from_slice(&(MAX_BATCH_SIGNATURES + 1).to_be_bytes()); | ||
| let result = run_mldsa65_batch_verify(u64::MAX, &input); | ||
| assert_eq!( | ||
| result.result, | ||
| InstructionResult::PrecompileError, | ||
| "C-1: expected PrecompileError for count > MAX_BATCH_SIGNATURES" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Added in a273ca7: batch_verify_multi_item_happy_path tests count=2 with two real ML-DSA-65/Dilithium3 keypairs and distinct messages — asserts all-1s output. batch_verify_multi_item_tampered_sig_fails uses the same setup but flips a byte in the second item's signature — asserts all-0s output. Both tests exercise the slicing fix directly.
- CI clippy: use saturating_mul instead of checked_mul+unwrap_or - precompiles.rs: fix per-item slicing in verify_mldsa65_batch — was passing &input[item_start..] (open-ended), causing verify to see signature + trailing bytes from subsequent items. Now slices to exact item end so count>1 batches verify correctly. - p2p_handlers.rs: reject WPoA votes when sig.sig_type != infer_signature_type_from_address(voter); prevents algorithm-tag confusion in commit certificates. - precompiles.rs tests: add multi-item batch verify regression test (count=2 happy path + one-tampered-sig path). - p2p_handlers.rs tests: add wpoa_handle_vote_rejects_sig_type_mismatch asserting rejection when sig.sig_type does not match inferred type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes from 2026-05-24 security audit.
Critical
handle_wpoa_view_changepattern. Unauthenticated votes no longer accepted.High
NodeError::Startupunless compiled with newstub-l2-verifierfeature (default=off). Startup banner warns loudly when stub is enabled.verify_mldsa65. TODO referencing audit L-2 left for Dilithium3 retirement.deny.tomladvisory acceptance comment for RUSTSEC-2026-0118/0119 with full mitigation rationale (DNS-only bootstrap, stall-not-corruption failure mode).cargo update -p hickory-protoconfirms no compatible patched release available for libp2p 0.56 yet.Tests
batch_verify_oog_before_verification_loop— gas_limit=0 + count=100 returns PrecompileOOG without panic.batch_verify_rejects_oversized_count— count > 256 returns PrecompileError immediately.wpoa_handle_vote_rejects_garbage_signature— two garbage-sig votes from known validators do not advance round past Voting.wpoa_handle_vote_reaches_quorumto use real DilithiumSigner key pairs and valid signatures.l2_source_binding_rejects_invalid_proof_bytes_without_stub— garbage proof_bytes returns Err in default build.l2_source_binding_accepts_settled_l1_sourcegated with#[cfg(feature = "stub-l2-verifier")].CI
cargo check --workspace✅cargo test -p shell-evm✅ (all tests pass)cargo test -p shell-node✅ (219 passed, 0 failed)cargo clippy --workspace --all-targets -- -D warningsshell-cryptocross_lang test andshell-stark-prover(present on main before this PR, not introduced here)Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com