fix(security): reject unsigned attestations in /attest/submit (fixes #8178) [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT]

[waterWang]

Issue

Closes #8178

Summary

The /attest/submit endpoint accepted attestations with empty signature and public_key fields, skipping all signature verification and auto-enrolling the wallet with full reward weight. This let an attacker with valid hardware enroll arbitrary wallets without proving ownership.

Fix

Added elif/else branches after the existing signature-verification block:

• elif sig_hex or pubkey_hex: → reject with INCOMPLETE_SIGNATURE (400) when only one of signature/public_key is present.
• else: → reject fully unsigned attestations with SIGNED_ATTESTATION_REQUIRED (401), matching the /epoch/enroll enforcement pattern.

Acceptance criteria

• Unsigned attestations (empty signature/public_key) are rejected with code SIGNED_ATTESTATION_REQUIRED
• Requests with only one of signature/public_key are rejected with INCOMPLETE_SIGNATURE
• Properly signed attestations still pass verification
• Added PoC test unsigned_attestation_poc.py verifying the fix

Wallet: fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT

[FlintLeng]

PR Review: Reject Unsigned Attestations in /attest/submit

Reviewed on: 2026-08-07

Summary

Critical security fix for CVE-class issue #8178. The /attest/submit endpoint had a backward-compatibility path that accepted empty signature and public_key fields, skipping Ed25519 verification entirely and auto-enrolling the wallet at full reward weight — without proving ownership of the wallet's private key.

Security Analysis ✅

The vulnerability is real and critical. An attacker who could reach the node's HTTP endpoint could submit an unsigned attestation for any wallet address and enroll it at the maximum reward tier. This is equivalent to a signature-bypass on enrollment — the entire reward weight system was undermined.

The three-branch rejection is correctly layered:

1. Missing both fields → HTTP 401 SIGNED_ATTESTATION_REQUIRED (not merely 400 — this is a security boundary, not a bad request)
2. Partial (only sig XOR pubkey) → HTTP 400 INCOMPLETE_SIGNATURE (malformed, not a security issue)
3. Proper Ed25519 signature verification unchanged

The HTTP status code choice is intentional and correct. 401 (Unauthorized) for fully unsigned attestations signals an authentication failure, not a validation error. 400 (Bad Request) for partial signatures correctly categorises it as a client-side malformed request. The distinction matters for security monitoring and rate-limiting.

The PoC test (unsigned_attestation_poc.py) validates the fix correctly. It checks source code presence of SIGNED_ATTESTATION_REQUIRED and elif sig_hex or pubkey_hex — appropriate for a regression test that doesn't require a live node. ENROLL_ALLOW_UNSIGNED_LEGACY = '0' in the test environment is the right toggle to force the new behaviour.

ENROLL_ALLOW_UNSIGNED_LEGACY = '0' as an environment variable suggests there's a way to enable the old (broken) behaviour. Worth confirming this flag is never set in production and that the default is 0 (rejecting). If it defaults to '1', this is a deployment risk.

Minor Note

The error message Re-attest with a signing key uses "signing key" — this is slightly imprecise. The fix is about proving ownership of the wallet's Ed25519 key pair, not a separate signing key. Consider Re-attest with your wallet's private key for clarity.

Wallet: RTC019e78d600fb3131c29d7ba80aba8fe644be426e

✅ LGTM — critical security fix, correctly implemented with appropriate PoC coverage.

[Scottcjn]

Holding this, and I want to be clear that the vulnerability is real. Accepting an attestation with no signature and no public key means no proof that the submitter controls the wallet, which is a Sybil vector rather than a theoretical one. It should be closed.

It cannot be closed as a hard cutover, though. As written this returns 401 for every attestation without signature and public_key, and the live fleet does not send them.

What I checked:

• The code path you are removing is a documented compatibility path. The comment directly above it at line 5039 reads: "Operators who intentionally run without pynacl can still accept unsigned attestations via the backward-compat path below (no signature fields, no verification attempted)." So unsigned is the currently supported case, not an oversight.
• No current miner client sends those fields. Grepping miners/ for "signature" and public_key outside deprecated/old_miners/ returns nothing.
• The node has 15 miners attesting right now, including PowerPC G4 and G5 machines on very old Python where adding Ed25519 signing is not a small change.

So merging this stops attestation for the entire fleet at once, and the vintage hardware is exactly the part that is hardest to upgrade. That is a chain halt for the miners rather than a hardening.

A migration that gets you the same end state:

1. Land the signature verification path and keep unsigned accepted, but mark unsigned attestations in the database and log them, so there is a real count of who still needs upgrading.
2. Ship signing in the miner clients, and confirm from that count that the fleet has moved over. The vintage miners will need the most lead time.
3. Gate enforcement behind a flag, defaulting to permissive, so the cutover is an operator decision and is reversible without a redeploy. The existing RC_TESTNET_ALLOW_MOCK_SIG and RC_TESTNET_ALLOW_INLINE_PUBKEY environment switches are the established pattern.
4. Flip the default once the count reaches zero.

Step 1 alone is a genuinely useful PR and I would take it now.

Two smaller notes:

• unsigned_attestation_poc.py is added at the repository root. PoC files belong beside the tests rather than at top level, and this repo has a pattern of PoC tests that assert the bug is present, which then fail once it is fixed. Worth writing it as a test that asserts the fixed behaviour instead.
• The elif sig_hex or pubkey_hex branch rejecting a half-supplied pair with 400 is correct and uncontroversial. That part could go in on its own immediately.

The finding is good work. It is the rollout that needs to change, not the analysis.