chore(sequencer): e2e tests for invalid signature recovery in checkpoint attestations

yarn-project/archiver/src/modules/validation.test.ts

@@ -2,7 +2,7 @@ import type { EpochCache } from '@aztec/epoch-cache';
 import { CheckpointNumber, EpochNumber, SlotNumber } from '@aztec/foundation/branded-types';
 import { Buffer32 } from '@aztec/foundation/buffer';
 import { times } from '@aztec/foundation/collection';
-import { Secp256k1Signer } from '@aztec/foundation/crypto/secp256k1-signer';
+import { Secp256k1Signer, flipSignature } from '@aztec/foundation/crypto/secp256k1-signer';
 import { Signature } from '@aztec/foundation/eth-signature';
 import { type Logger, createLogger } from '@aztec/foundation/log';
 import { CommitteeAttestation, EthAddress } from '@aztec/stdlib/block';
@@ -153,6 +153,27 @@ describe('validateCheckpointAttestations', () => {
       expect(result.invalidIndex).toBe(0);
     });
 
+    it('fails if an attestation signature has a high-s value (malleable signature)', async () => {
+      const checkpoint = await makeCheckpoint(signers.slice(0, 4), committee);
+
+      // Flip the signature at index 2 to give it a high-s value
+      const original = checkpoint.attestations[2];
+      const flipped = flipSignature(original.signature);
+      checkpoint.attestations[2] = new CommitteeAttestation(original.address, flipped);
+
+      // Verify the flipped signature is detected as invalid
+      const attestations = getAttestationInfoFromPublishedCheckpoint(checkpoint);
+      expect(attestations[2].status).toBe('invalid-signature');
+
+      const result = await validateCheckpointAttestations(checkpoint, epochCache, constants, logger);
+      assert(!result.valid);
+      assert(result.reason === 'invalid-attestation');
+      expect(result.checkpoint.checkpointNumber).toEqual(checkpoint.checkpoint.number);
+      expect(result.checkpoint.archive.toString()).toEqual(checkpoint.checkpoint.archive.root.toString());
+      expect(result.committee).toEqual(committee);
+      expect(result.invalidIndex).toBe(2);
+    });
+
     it('reports correct index when invalid attestation follows provided address', async () => {
       const checkpoint = await makeCheckpoint(signers.slice(0, 3), committee);
 

yarn-project/foundation/src/crypto/secp256k1-signer/malleability.test.ts

@@ -8,6 +8,8 @@ import { Secp256k1Signer } from './secp256k1_signer.js';
 import {
   Secp256k1Error,
   flipSignature,
+  generateRecoverableSignature,
+  generateUnrecoverableSignature,
   makeEthSignDigest,
   normalizeSignature,
   recoverAddress,
@@ -139,3 +141,21 @@ describe('ecdsa malleability', () => {
     expect(recoveredAddress.toString()).toEqual(expectedAddress.toString());
   });
 });
+
+describe('generateRecoverableSignature', () => {
+  it('produces a signature from which an address can be recovered', () => {
+    const sig = generateRecoverableSignature();
+    const hash = Buffer32.random();
+    const recovered = tryRecoverAddress(hash, sig);
+    expect(recovered).toBeDefined();
+  });
+});
+
+describe('generateUnrecoverableSignature', () => {
+  it('produces a signature from which no address can be recovered', () => {
+    const sig = generateUnrecoverableSignature();
+    const hash = Buffer32.random();
+    const recovered = tryRecoverAddress(hash, sig);
+    expect(recovered).toBeUndefined();
+  });
+});

yarn-project/foundation/src/crypto/secp256k1-signer/utils.ts

@@ -210,3 +210,35 @@ export function recoverPublicKey(hash: Buffer32, signature: Signature, opts: Rec
   const publicKey = sig.recoverPublicKey(hash.buffer).toHex(false);
   return Buffer.from(publicKey, 'hex');
 }
+
+/** Arbitrary hash used for testing signature recoverability. */
+const PROBE_HASH = Buffer32.fromBuffer(keccak256(Buffer.from('signature-recoverability-probe')));
+
+/**
+ * Generates a random valid ECDSA signature that is recoverable to some address.
+ * Since Signature.random() produces real signatures via secp256k1 signing, the result is always
+ * recoverable, but we verify defensively by checking tryRecoverAddress.
+ */
+export function generateRecoverableSignature(): Signature {
+  for (let i = 0; i < 100; i++) {
+    const sig = Signature.random();
+    if (tryRecoverAddress(PROBE_HASH, sig) !== undefined) {
+      return sig;
+    }
+  }
+  throw new Secp256k1Error('Failed to generate a recoverable signature after 100 attempts');
+}
+
+/**
+ * Generates a random signature where ECDSA address recovery fails.
+ * Uses random r/s values (not from real signing) so that r is unlikely to be a valid secp256k1 x-coordinate.
+ */
+export function generateUnrecoverableSignature(): Signature {
+  for (let i = 0; i < 100; i++) {
+    const sig = new Signature(Buffer32.random(), Buffer32.random(), 27);
+    if (tryRecoverAddress(PROBE_HASH, sig) === undefined) {
+      return sig;
+    }
+  }
+  throw new Secp256k1Error('Failed to generate an unrecoverable signature after 100 attempts');
+}

yarn-project/foundation/src/eth-signature/eth_signature.test.ts

@@ -1,7 +1,9 @@
 import { Buffer32 } from '@aztec/foundation/buffer';
-import { Secp256k1Signer, recoverAddress } from '@aztec/foundation/crypto/secp256k1-signer';
+import { Secp256k1Signer, recoverAddress, tryRecoverAddress } from '@aztec/foundation/crypto/secp256k1-signer';
 import { Fr } from '@aztec/foundation/curves/bn254';
 
+import { secp256k1 } from '@noble/curves/secp256k1';
+
 import { Signature } from './eth_signature.js';
 
 const randomSigner = () => {
@@ -62,4 +64,24 @@ describe('eth signature', () => {
     const deserialized = Signature.fromString(serialized);
     checkEquivalence(signature, deserialized);
   });
+
+  it('random() produces a valid recoverable signature with low s-value', () => {
+    const sig = Signature.random();
+
+    // v should be 27 or 28
+    expect([27, 28]).toContain(sig.v);
+
+    // Signature should not be empty
+    expect(sig.isEmpty()).toBe(false);
+
+    // s should be in the low half of the curve (low s-value)
+    const sBigInt = sig.s.toBigInt();
+    const halfN = secp256k1.CURVE.n / 2n;
+    expect(sBigInt).toBeLessThanOrEqual(halfN);
+
+    // Signature should be recoverable (tryRecoverAddress should return an address for any hash)
+    const hash = Buffer32.random();
+    const recovered = tryRecoverAddress(hash, sig);
+    expect(recovered).toBeDefined();
+  });
 });

yarn-project/foundation/src/eth-signature/eth_signature.ts

@@ -1,8 +1,10 @@
 import { Buffer32 } from '@aztec/foundation/buffer';
 import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize';
 
+import { secp256k1 } from '@noble/curves/secp256k1';
 import { z } from 'zod';
 
+import { randomBytes } from '../crypto/random/index.js';
 import { hasHexPrefix, hexToBuffer } from '../string/index.js';
 
 /**
@@ -77,8 +79,12 @@ export class Signature {
     return new Signature(Buffer32.fromBuffer(hexToBuffer(sig.r)), Buffer32.fromBuffer(hexToBuffer(sig.s)), sig.yParity);
   }
 
+  /** Generates a random valid ECDSA signature with a low s-value by signing a random message with a random key. */
   static random(): Signature {
-    return new Signature(Buffer32.random(), Buffer32.random(), 1);
+    const privateKey = randomBytes(32);
+    const message = randomBytes(32);
+    const { r, s, recovery } = secp256k1.sign(message, privateKey);
+    return new Signature(Buffer32.fromBigInt(r), Buffer32.fromBigInt(s), recovery ? 28 : 27);
   }
 
   static empty(): Signature {

yarn-project/sequencer-client/src/config.ts

@@ -52,6 +52,8 @@ export const DefaultSequencerConfig: ResolvedSequencerConfig = {
   skipInvalidateBlockAsProposer: false,
   broadcastInvalidBlockProposal: false,
   injectFakeAttestation: false,
+  injectHighSValueAttestation: false,
+  injectUnrecoverableSignatureAttestation: false,
   fishermanMode: false,
   shuffleAttestationOrdering: false,
   skipPushProposedBlocksToArchiver: false,
@@ -186,6 +188,14 @@ export const sequencerConfigMappings: ConfigMappingsType<SequencerConfig> = {
     description: 'Inject a fake attestation (for testing only)',
     ...booleanConfigHelper(DefaultSequencerConfig.injectFakeAttestation),
   },
+  injectHighSValueAttestation: {
+    description: 'Inject a malleable attestation with a high-s value (for testing only)',
+    ...booleanConfigHelper(DefaultSequencerConfig.injectHighSValueAttestation),
+  },
+  injectUnrecoverableSignatureAttestation: {
+    description: 'Inject an attestation with an unrecoverable signature (for testing only)',
+    ...booleanConfigHelper(DefaultSequencerConfig.injectUnrecoverableSignatureAttestation),
+  },
   fishermanMode: {
     env: 'FISHERMAN_MODE',
     description:

yarn-project/sequencer-client/src/sequencer/checkpoint_proposal_job.ts

@@ -9,6 +9,11 @@ import {
   SlotNumber,
 } from '@aztec/foundation/branded-types';
 import { randomInt } from '@aztec/foundation/crypto/random';
+import {
+  flipSignature,
+  generateRecoverableSignature,
+  generateUnrecoverableSignature,
+} from '@aztec/foundation/crypto/secp256k1-signer';
 import { Fr } from '@aztec/foundation/curves/bn254';
 import { EthAddress } from '@aztec/foundation/eth-address';
 import { Signature } from '@aztec/foundation/eth-signature';
@@ -759,7 +764,12 @@ export class CheckpointProposalJob implements Traceable {
       const sorted = orderAttestations(trimmed, committee);
 
       // Manipulate the attestations if we've been configured to do so
-      if (this.config.injectFakeAttestation || this.config.shuffleAttestationOrdering) {
+      if (
+        this.config.injectFakeAttestation ||
+        this.config.injectHighSValueAttestation ||
+        this.config.injectUnrecoverableSignatureAttestation ||
+        this.config.shuffleAttestationOrdering
+      ) {
         return this.manipulateAttestations(proposal.slotNumber, epoch, seed, committee, sorted);
       }
 
@@ -788,7 +798,11 @@ export class CheckpointProposalJob implements Traceable {
       this.epochCache.computeProposerIndex(slotNumber, epoch, seed, BigInt(committee.length)),
     );
 
-    if (this.config.injectFakeAttestation) {
+    if (
+      this.config.injectFakeAttestation ||
+      this.config.injectHighSValueAttestation ||
+      this.config.injectUnrecoverableSignatureAttestation
+    ) {
       // Find non-empty attestations that are not from the proposer
       const nonProposerIndices: number[] = [];
       for (let i = 0; i < attestations.length; i++) {
@@ -798,8 +812,20 @@ export class CheckpointProposalJob implements Traceable {
       }
       if (nonProposerIndices.length > 0) {
         const targetIndex = nonProposerIndices[randomInt(nonProposerIndices.length)];
-        this.log.warn(`Injecting fake attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`);
-        unfreeze(attestations[targetIndex]).signature = Signature.random();
+        if (this.config.injectHighSValueAttestation) {
+          this.log.warn(
+            `Injecting high-s value attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`,
+          );
+          unfreeze(attestations[targetIndex]).signature = flipSignature(attestations[targetIndex].signature);
+        } else if (this.config.injectUnrecoverableSignatureAttestation) {
+          this.log.warn(
+            `Injecting unrecoverable signature attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`,
+          );
+          unfreeze(attestations[targetIndex]).signature = generateUnrecoverableSignature();
+        } else {
+          this.log.warn(`Injecting fake attestation in checkpoint for slot ${slotNumber} at index ${targetIndex}`);
+          unfreeze(attestations[targetIndex]).signature = generateRecoverableSignature();
+        }
       }
       return new CommitteeAttestationsAndSigners(attestations);
     }

yarn-project/stdlib/src/interfaces/configs.ts

@@ -59,6 +59,10 @@ export interface SequencerConfig {
   broadcastInvalidBlockProposal?: boolean;
   /** Inject a fake attestation (for testing only) */
   injectFakeAttestation?: boolean;
+  /** Inject a malleable attestation with a high-s value (for testing only) */
+  injectHighSValueAttestation?: boolean;
+  /** Inject an attestation with an unrecoverable signature (for testing only) */
+  injectUnrecoverableSignatureAttestation?: boolean;
   /** Whether to run in fisherman mode: builds blocks on every slot for validation without publishing */
   fishermanMode?: boolean;
   /** Shuffle attestation ordering to create invalid ordering (for testing only) */
@@ -104,6 +108,8 @@ export const SequencerConfigSchema = zodFor<SequencerConfig>()(
     secondsBeforeInvalidatingBlockAsNonCommitteeMember: z.number(),
     broadcastInvalidBlockProposal: z.boolean().optional(),
     injectFakeAttestation: z.boolean().optional(),
+    injectHighSValueAttestation: z.boolean().optional(),
+    injectUnrecoverableSignatureAttestation: z.boolean().optional(),
     fishermanMode: z.boolean().optional(),
     shuffleAttestationOrdering: z.boolean().optional(),
     blockDurationMs: z.number().positive().optional(),