Skip to content

fix(p2p): canonicalize yParity attestation signatures before L1 bundle - #24515

Merged
PhilWindle merged 2 commits into
merge-train/spartan-v5from
spl/a-1351-sequencerstdlib-malicious-v0-yparity-attestation-makes
Jul 5, 2026
Merged

fix(p2p): canonicalize yParity attestation signatures before L1 bundle#24515
PhilWindle merged 2 commits into
merge-train/spartan-v5from
spl/a-1351-sequencerstdlib-malicious-v0-yparity-attestation-makes

Conversation

@spalladino

@spalladino spalladino commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Context

A committee member — or any peer mutating the recovery byte in flight — can emit an attestation signature in yParity form (v = 0/1) that still recovers to the same signer, since coordination-signature recovery allows yParity as v. This is enough to brick an honest proposer.

On the proposer side, CommitteeAttestationsAndSigners.packAttestations treated v === 0 as an empty (address-only) slot, while getSigners treats a signature as empty only when r, s, and v are all zero. The two disagree for a real signature with v = 0: the packed bitmap popcount excludes it but the signers array keeps it, so an honest proposer's propose() reverts on L1 with AttestationLib__SignersSizeMismatch. A v = 1 signature instead passes propose() but reverts epoch proving, where verifyAttestations runs ECDSA.recover and rejects v ∉ {27,28}. Either variant lets a single actor repeatably halt block production or proving for slots they attest to, with no slashing signal (they did attest).

Approach

Canonicalize the attester signature's recovery byte to v ∈ {27,28} (which keeps (r, s) and the recovery bit, so the recovered address and the signer's vote are preserved), at three layers:

  • orderAttestations — the honest path that assembles the L1 bundle, via the existing normalizeSignature. It runs before the adversarial-injection manipulation used by the invalid-attestation e2e tests, so those deliberately-malformed signatures are left untouched.
  • Attestation pool ingress (tryAddCheckpointAttestation and addOwnCheckpointAttestations) — defense in depth, so stored and re-gossiped bytes are always canonical.
  • CommitteeAttestationsAndSigners.packAttestations itself — its empty-slot check now matches getSigners() (r, s, v all zero, not just v === 0), and it canonicalizes a yParity recovery byte (0/1 → 27/28) when packing. This rewrites only the v byte — it does not touch (r, s) and leaves any other v value as-is — so the injectHighSValueAttestation / injectUnrecoverableSignatureAttestation / MaliciousCommitteeAttestationsAndSigners paths (which inject malformed signatures after orderAttestations to verify L1 rejection) still behave as before. A full normalizeSignature inside the class would break them; the v-byte-only fix does not.

Regression tests cover both v = 0 and v = 1 at the orderAttestations layer, the pool ingress layer, and directly on packAttestations (each asserted red first, e.g. bitmap popcount 3/4 vs signers 4, then green).

Fixes A-1351

A committee member (or any peer mutating the recovery byte in flight) can emit an
attestation signature in yParity form (v = 0/1) that still recovers to the same signer,
because coordination-signature recovery allows yParity as v. On the proposer side,
`CommitteeAttestationsAndSigners.packAttestations` treats v === 0 as an empty (address-only)
slot while `getSigners` treats a signature as empty only when r, s and v are all zero. The two
disagree for a real signature with v = 0, so the bitmap popcount and the signers array diverge
and an honest proposer's `propose()` reverts with AttestationLib__SignersSizeMismatch. A v = 1
signature instead passes propose() but reverts epoch proving, where verifyAttestations runs
ECDSA.recover and rejects v not in {27,28}. Either way a single actor can repeatably brick block
production or proving for slots they attest to.

Canonicalize the recovery byte to v in {27,28} in `orderAttestations` — the honest path that
builds the L1 bundle, running before any adversarial-injection manipulation — and on ingress to
the attestation pool as defense in depth. Normalization keeps (r, s) and the recovery bit, so the
recovered address (and the signer's vote) is preserved.
…ures

Defense in depth for the yParity attestation DoS. `packAttestations` classified a
slot as empty on `v === 0` alone, while `getSigners` uses the full empty check
(r, s and v all zero); the two disagreed for a signature with v = 0 and non-zero
(r, s), so the bitmap popcount and the signers array diverged. Align the empty
check to r, s and v all zero, and canonicalize a yParity recovery byte (v = 0/1)
to 27/28 when packing so a signature that reaches the class already in yParity form
is still consistent and accepted by L1 ECDSA.recover at proving time. Any other v is
left untouched so a genuinely malformed signature still fails on L1.

Does not touch (r, s), so the injectHighSValueAttestation /
injectUnrecoverableSignatureAttestation adversarial paths keep producing the
malformed signatures they rely on to verify L1 rejection.
@PhilWindle
PhilWindle merged commit c545f94 into merge-train/spartan-v5 Jul 5, 2026
15 checks passed
@PhilWindle
PhilWindle deleted the spl/a-1351-sequencerstdlib-malicious-v0-yparity-attestation-makes branch July 5, 2026 18:02
PhilWindle pushed a commit that referenced this pull request Jul 6, 2026
…(A-1401) (#24537)

## Context

A-1351 (#24515) fixed the honest-proposer and relay path for
non-canonical yParity attestation signatures, but a malicious selected
proposer can still hand-craft `propose()` calldata carrying a
non-proposer signature slot with `v ∈ {0,1}` (or an all-zero `(r,s,v)`
in a bitmap-marked signature slot). L1 accepts this at propose time (it
only recovers the proposer's own slot) and stores the attestations hash
over the raw bytes. Afterwards, either:

- the slot recovers to the correct member, so honest nodes deem the
checkpoint valid and nobody invalidates it — but epoch proving reverts
at `ECDSA.recover` (which rejects `v ∉ {27,28}`), a silent stall; or
- the slot is detected as invalid, but the invalidation repack via
`packAttestations` is not byte-faithful, so the recomputed hash diverges
from the stored one and `InvalidateLib` reverts — wedging the chain
until prune.

This is the malicious-proposer sibling of A-1351; it survives that fix.
Tracks aztec-claude#650.

## Approach

TS-only — no L1 changes, since this targets a fresh deployment. Two
parts:

- **Detection** — `getAttestationInfoFromPayload` no longer passes
`allowYParityAsV: true`, so TS signature validity matches L1's
`ECDSA.recover`. A non-canonical slot is now classified as an invalid
attestation, so honest nodes detect the bad checkpoint instead of
silently accepting it.
- **Byte-faithful invalidation** — the raw packed
`CommitteeAttestations` tuple from the checkpoint calldata is threaded
verbatim (as `verbatimAttestations`) through the negative
`ValidateCheckpointResult` into `buildInvalidateCheckpointRequest`,
which submits those exact bytes rather than repacking. With the original
bytes, `invalidateBadAttestation` reproduces the stored hash and L1's
`tryRecover` returns `address(0) ≠ member`, so invalidation succeeds.

`verbatimAttestations` is a required field on the negative result and is
serialized unconditionally — a repack fallback is intentionally absent,
since silently repacking would re-introduce the wedge.

The gossip sender-recovery path (`recoverCoordinationSigner`)
deliberately stays lenient (`allowYParityAsV`) so an honest node can
still attribute a yParity-encoded message and canonicalize it on
ingress; only the on-chain checkpoint validation is made strict. A
comment documents the split so it is not accidentally unified.

## Tests

An e2e regression under the invalidation suite
(`invalidate_block.parallel.test.ts`, "proposer invalidates checkpoint
with a yParity attestation slot") demonstrates the attack red→green: a
malicious proposer lands a checkpoint whose non-proposer attestation
slots carry raw yParity bytes, and the honest next proposer invalidates
it — which only succeeds when the invalidation submits the verbatim
calldata bytes (a repack diverges from the on-chain `attestationsHash`
and reverts). Plus unit coverage for the strict attestation-info
classification, the byte-faithful passthrough, and the result
round-trip.

An attester-side variant (a committee member emitting yParity
attestations) is not separately tested: the honest proposer's
`orderAttestations` normalization (A-1351) canonicalizes the byte before
the L1 bundle, so it never reaches L1 and produces the same on-chain
outcome as — and is subsumed by — the malicious-proposer case above.

Fixes A-1401
rangozd pushed a commit to rangozd/aztec-packages that referenced this pull request Aug 5, 2026
Clean forward-port of AztecProtocol#24524 (merge-train
spartan-v5, merge commit f397aa5, parents aba2a2e..7e7d8d0) onto
cb/private-port-next-7. Cherry-pick -m 1 applied with zero conflicts;
resulting tree touches exactly the 152 files of the source merge diff.

Pure TS/docs/CI (117 e2e-consolidation test files + sequencer-client,
archiver, stdlib TS, p2p, ethereum, validator-client, foundation, docs,
.test_patterns.yml). No noir/circuit/pinned-artifact surfaces → no
standard-contracts regen. Bundles sub-PRs AztecProtocol#24504 AztecProtocol#24506 AztecProtocol#24507 AztecProtocol#24509
AztecProtocol#24513 AztecProtocol#24515 AztecProtocol#24474 AztecProtocol#24489 AztecProtocol#24490 AztecProtocol#24491 AztecProtocol#24492 AztecProtocol#24494 AztecProtocol#24495 AztecProtocol#24498
AztecProtocol#24500 AztecProtocol#24503 AztecProtocol#24475.

Tracker: AztecProtocol/aztec-packages-private#570
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants