fix(security): prevent signature replay attack in QC verification

[SherlockShemol]

Fix Signature Scope Attack Vulnerability

Summary

This PR fixes a critical security vulnerability where attackers can forge QuorumCertificates (QCs) with arbitrary view numbers by replaying signatures from legitimate QCs.

Problem

The VerifyQuorumCert function in security/cert/auth.go does not validate that QC.View matches Block.View. This allows an attacker to:

1. Take signatures from a legitimate QC (e.g., View 10)
2. Create a fake QC with the same signatures but a different View (e.g., View 100)
3. The fake QC passes verification because signatures only cover block.ToBytes()

Attack Illustration

LEGITIMATE QC (View 10):
┌─────────────────────────────────┐
│  View: 10                       │
│  BlockHash: 0xABC...            │
│  Signatures: [sig1, sig2, ...]  │  ← Signatures cover block content
└─────────────────────────────────┘
                │
                │ Attacker copies signatures
                ▼
FORGED QC (View 100):
┌─────────────────────────────────┐
│  View: 100  ← TAMPERED!         │
│  BlockHash: 0xABC...            │
│  Signatures: [sig1, sig2, ...]  │  ← Same signatures, still valid!
└─────────────────────────────────┘
                │
                │ Verification PASSES ❌
                ▼
┌─────────────────────────────────┐
│  Node updates HighQC to View 100│
│  Consensus disrupted!           │
└─────────────────────────────────┘

Real-World Impact

Scenario	Consequence
HighQC Manipulation	Attacker inflates HighQC.View from 3 to 999
Time-Warp Proposal	Attacker creates proposals for arbitrary future views
View Synchronization	Nodes desynchronize, causing liveness issues
Consensus Stall	Legitimate proposals rejected due to "stale" views

Solution

Add a simple check to ensure QC.View == Block.View:

 func (c *Authority) VerifyQuorumCert(qc hotstuff.QuorumCert) error {
     // ... existing checks ...

     block, ok := c.blockchain.Get(qc.BlockHash())
     if !ok {
         return fmt.Errorf("block not found: %v", qc.BlockHash())
     }
+
+    // Prevent signature replay attacks: QC.View must match Block.View.
+    if qc.View() != block.View() {
+        return fmt.Errorf("QC view %d does not match block view %d (possible signature replay attack)",
+            qc.View(), block.View())
+    }

     return c.Verify(qc.Signature(), block.ToBytes())
 }

Why This Fix Is Correct

1. All Legitimate QCs Satisfy This Check

The CreateQuorumCert function already sets QC.View = block.View():

func (c *Authority) CreateQuorumCert(block *hotstuff.Block, signatures []hotstuff.PartialCert) (cert hotstuff.QuorumCert, err error) {
    // ...
    return hotstuff.NewQuorumCert(sig, block.View(), block.Hash()), nil
    //                                ^^^^^^^^^^^^
}

2. No Breaking Changes

Aspect	Impact
Signature format	Unchanged
Network protocol	Unchanged
API	Unchanged
Existing tests	All pass

3. Performance

• Before: O(signature_verification)
• After: O(signature_verification) + O(1) (one integer comparison)
• Impact: Negligible

Testing

New Test Added

Test	Description
TestVerifyQuorumCert_SignatureReplayAttack	Core vulnerability test

Test Results (Before Fix)

=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/ecdsa
    VULNERABILITY: QC with tampered View was accepted
=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/eddsa
    VULNERABILITY: QC with tampered View was accepted
=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/bls12
    VULNERABILITY: QC with tampered View was accepted
--- FAIL: TestVerifyQuorumCert_SignatureReplayAttack

Test Results (After Fix)

=== RUN   TestVerifyQuorumCert_SignatureReplayAttack
=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/ecdsa
=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/eddsa
=== RUN   TestVerifyQuorumCert_SignatureReplayAttack/bls12
--- PASS: TestVerifyQuorumCert_SignatureReplayAttack (0.01s)

Full Test Suite

$ go test ./...
ok      github.com/relab/hotstuff/security/cert     (all pass)
ok      github.com/relab/hotstuff/protocol/...      (all pass)

Checklist

• Code compiles without errors
• All existing tests pass
• New test added for the vulnerability
• Fix verified across ECDSA, EdDSA, and BLS12
• No breaking changes to API or protocol
• Documentation updated (code comments)

Related Issues

• Closes Security: Signature Scope Attack (QC View Not Protected) #289 (Signature Scope Attack Vulnerability)

[leandernikolaus]

Hi,

Thank for contributing this.
The fix looks good, and it may be relevant to keep some of the test cases, but probably not all.
Can you explain what is the benefit of keeping the TimeWarp, HighQCManipulation and DiagnosticInfo tests?
Also, could you try to reduce the verbosity in the test cases and the comments?

Furthermore, it does not appear that the test cases utilize the Twins framework. Is that correct?
Could this simply be a unit test on the different crypto implementations?

[SherlockShemol]

Hi,

Thank for contributing this. The fix looks good, and it may be relevant to keep some of the test cases, but probably not all. Can you explain what is the benefit of keeping the TimeWarp, HighQCManipulation and DiagnosticInfo tests? Also, could you try to reduce the verbosity in the test cases and the comments?

Furthermore, it does not appear that the test cases utilize the Twins framework. Is that correct? Could this simply be a unit test on the different crypto implementations?

Hi @leandernikolaus ,
Thank you for the feedback! I've updated the PR accordingly:

• Simplified the test - Reduced from 4 tests to 1 core test (TestVerifyQuorumCert_SignatureReplayAttack), removing the redundant TimeWarp, HighQCManipulation, and DiagnosticInfo tests.
• Moved test to correct location - Since this is essentially a unit test for VerifyQuorumCert and doesn't use the Twins framework, I moved it from twins/ to security/cert/.