diff --git a/security/cert/auth.go b/security/cert/auth.go index 7c21b01f..4a9469bd 100644 --- a/security/cert/auth.go +++ b/security/cert/auth.go @@ -134,6 +134,17 @@ func (c *Authority) VerifyQuorumCert(qc hotstuff.QuorumCert) error { if !ok { return fmt.Errorf("block not found: %v", qc.BlockHash()) } + + // Prevent signature replay attacks: QC.View must match Block.View. + // Without this check, an attacker could take signatures from a legitimate QC + // (e.g., View 10) and create a fake QC claiming a much higher view (e.g., View 100). + // Since signatures only cover block.ToBytes() (which includes block.View), + // changing QC.View alone would not invalidate the signature. + // This allows "time-warp" attacks that can manipulate HighQC and disrupt consensus. + 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()) } diff --git a/security/cert/signature_scope_test.go b/security/cert/signature_scope_test.go new file mode 100644 index 00000000..ee587c8b --- /dev/null +++ b/security/cert/signature_scope_test.go @@ -0,0 +1,68 @@ +package cert_test + +import ( + "testing" + + "github.com/relab/hotstuff" + "github.com/relab/hotstuff/internal/testutil" + "github.com/relab/hotstuff/security/crypto" +) + +// TestVerifyQuorumCert_SignatureReplayAttack tests that QC verification rejects +// forged QCs where the View field has been tampered while reusing valid signatures. +// +// Attack scenario: +// 1. Attacker obtains a legitimate QC for Block A at View 10 +// 2. Attacker creates a fake QC with: same signatures, same BlockHash, but View=100 +// 3. If verification only checks block content (not view), the fake QC passes +// +// This test ensures the fix in VerifyQuorumCert correctly validates QC.View == Block.View. +func TestVerifyQuorumCert_SignatureReplayAttack(t *testing.T) { + for _, cryptoName := range []string{crypto.NameECDSA, crypto.NameEDDSA, crypto.NameBLS12} { + t.Run(cryptoName, func(t *testing.T) { + const numReplicas = 4 + const originalView = hotstuff.View(10) + const fakeView = hotstuff.View(100) + + // Setup: Create a legitimate QC at View 10 + dummies := testutil.NewEssentialsSet(t, numReplicas, cryptoName) + signers := dummies.Signers() + + genesisQC := hotstuff.NewQuorumCert(nil, 0, hotstuff.GetGenesis().Hash()) + block := hotstuff.NewBlock( + hotstuff.GetGenesis().Hash(), + genesisQC, + nil, + originalView, + hotstuff.ID(1), + ) + + for _, dummy := range dummies { + dummy.Blockchain().Store(block) + } + + // Create legitimate QC + legitimateQC := testutil.CreateQC(t, block, signers...) + + // Verify original QC is valid + if err := signers[0].VerifyQuorumCert(legitimateQC); err != nil { + t.Fatalf("Legitimate QC should be valid: %v", err) + } + + // Attack: Create fake QC with tampered View but same signatures + fakeQC := hotstuff.NewQuorumCert( + legitimateQC.Signature(), + fakeView, // Tampered: View 10 -> View 100 + legitimateQC.BlockHash(), + ) + + // Verification should fail for the tampered QC + err := signers[0].VerifyQuorumCert(fakeQC) + if err == nil { + t.Errorf("VULNERABILITY: QC with tampered View (%d->%d) was accepted; "+ + "VerifyQuorumCert should reject QCs where QC.View != Block.View", + originalView, fakeView) + } + }) + } +}