Skip to content

Latest commit

 

History

History
277 lines (229 loc) · 13.6 KB

074.md

File metadata and controls

277 lines (229 loc) · 13.6 KB

Macho Merlot Squirrel

Medium

Finality Provider will Avoid Slashing by Manipulating Transaction Ordering

Summary

A race condition in the finality module's evidence processing logic will cause an unfair slashing outcome for finality providers as malicious finality providers will submit their signatures in a specific order to avoid penalties for double-signing.

Root Cause

In x/finality/keeper/msg_server.go, the slashing behavior for double-signing depends on transaction ordering rather than just the presence of both signatures:

  1. When a fork signature is processed first followed by a canonical signature, slashing occurs as expected
  2. When a canonical signature is processed first followed by a fork signature, the fork signature is rejected as "duplicated finality vote" without triggering slashing

Internal Pre-conditions

  1. A finality provider must create two different signatures for the same block height (double-signing)
  2. The finality provider must be able to control the submission order of these signatures

External Pre-conditions

None

Attack Path

  1. A malicious finality provider signs two different blocks at the same height (canonical and fork)
  2. The provider ensures the canonical signature reaches the network first
  3. When the fork signature is submitted later, it's rejected as a "duplicated finality vote" rather than treated as equivocation evidence
  4. The provider avoids slashing penalties despite having double-signed

Impact

The protocol suffers a security weakening as the slashing mechanism for equivocation becomes inconsistent. Finality providers can game the system by strategically controlling transaction ordering, undermining the network's security model of immediate punishment for misbehavior.

PoC

// file location: babylon/x/finality/keeper/msg_server_test.go

func TestSlashingOrderDependence(t *testing.T) {
	// This test demonstrates a critical security vulnerability where the system's
	// slashing behavior depends on the order in which conflicting signatures are processed.
	// A finality provider who signs two different blocks at the same height should
	// always be detected and slashed, regardless of which signature is processed first.

	// SCENARIO 1: Fork signature first, canonical second
	t.Run("Fork signature first", func(t *testing.T) {
		// Setup with fresh state
		ctrl := gomock.NewController(t)
		defer ctrl.Finish()

		bsKeeper := types.NewMockBTCStakingKeeper(ctrl)
		bsKeeper.EXPECT().UpdateFinalityProvider(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
		cKeeper := types.NewMockCheckpointingKeeper(ctrl)
		iKeeper := types.NewMockIncentiveKeeper(ctrl)
		iKeeper.EXPECT().IndexRefundableMsg(gomock.Any(), gomock.Any()).AnyTimes()
		fKeeper, ctx := keepertest.FinalityKeeper(t, bsKeeper, iKeeper, cKeeper)
		ms := keeper.NewMsgServerImpl(*fKeeper)

		// Setup finality provider
		r := rand.New(rand.NewSource(0))
		btcSK, btcPK, err := datagen.GenRandomBTCKeyPair(r)
		require.NoError(t, err)
		fpBTCPK := bbn.NewBIP340PubKeyFromBTCPK(btcPK)
		fpBTCPKBytes := fpBTCPK.MustMarshal()
		fp := &bstypes.FinalityProvider{
			BtcPk: fpBTCPK,
		}

		// Generate test data
		signer := fp.Addr
		startHeight := uint64(10)
		blockHeight := uint64(15)
		canonicalAppHash := datagen.GenRandomByteArray(rand.New(rand.NewSource(0)), 32)
		forkAppHash := datagen.GenRandomByteArray(rand.New(rand.NewSource(1)), 32)

		// Setup mock expectations
		bsKeeper.EXPECT().HasFinalityProvider(gomock.Any(), gomock.Any()).Return(true).AnyTimes()
		cKeeper.EXPECT().GetEpoch(gomock.Any()).Return(&epochingtypes.Epoch{EpochNumber: 1}).AnyTimes()
		cKeeper.EXPECT().GetLastFinalizedEpoch(gomock.Any()).Return(uint64(2)).AnyTimes()
		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), gomock.Any()).Return(uint64(1)).AnyTimes()

		// Setup block context
		ctx = ctx.WithHeaderInfo(header.Info{Height: int64(blockHeight), AppHash: canonicalAppHash})
		fKeeper.IndexBlock(ctx)
		fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 1)

		// Commit public randomness
		randListInfo, msgCommitPubRandList, err := datagen.GenRandomMsgCommitPubRandList(r, btcSK, startHeight, uint64(200))
		require.NoError(t, err)
		_, err = ms.CommitPubRandList(ctx, msgCommitPubRandList)
		require.NoError(t, err)

		// Create signature messages
		forkMsg, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, forkAppHash)
		require.NoError(t, err)
		canonicalMsg, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, canonicalAppHash)
		require.NoError(t, err)

		// Track slashing calls
		slashCalled := false
		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(2)
		bsKeeper.EXPECT().SlashFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).DoAndReturn(
			func(ctx sdk.Context, btcPk []byte) (uint64, error) {
				slashCalled = true
				return 100, nil
			}).Times(1)

		// VULNERABILITY PART 1: Process fork signature first
		// When fork signature is processed first, the system stores it without slashing
		t.Log("Processing fork signature first...")
		_, err = ms.AddFinalitySig(ctx, forkMsg)
		require.NoError(t, err)
		require.False(t, slashCalled, "Slashing should not occur after just the fork signature")
		t.Log("Fork signature processed successfully, no slashing occurred yet")

		// When canonical signature comes second, the system correctly detects conflicting signatures
		// and slashes the finality provider - this is expected behavior
		t.Log("Processing canonical signature second...")
		_, err = ms.AddFinalitySig(ctx, canonicalMsg)
		require.NoError(t, err)
		require.True(t, slashCalled, "Slashing should occur after both signatures are processed")
		t.Log("Canonical signature processed, slashing occurred as expected")
	})

	// SCENARIO 2: Canonical signature first, fork second
	t.Run("Canonical signature first", func(t *testing.T) {
		// Use completely separate test setup to avoid state carryover
		ctrl := gomock.NewController(t)
		defer ctrl.Finish()

		bsKeeper := types.NewMockBTCStakingKeeper(ctrl)
		bsKeeper.EXPECT().UpdateFinalityProvider(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
		cKeeper := types.NewMockCheckpointingKeeper(ctrl)
		iKeeper := types.NewMockIncentiveKeeper(ctrl)
		iKeeper.EXPECT().IndexRefundableMsg(gomock.Any(), gomock.Any()).AnyTimes()
		fKeeper, ctx := keepertest.FinalityKeeper(t, bsKeeper, iKeeper, cKeeper)
		ms := keeper.NewMsgServerImpl(*fKeeper)

		// Setup finality provider with different seed
		r := rand.New(rand.NewSource(2))
		btcSK, btcPK, err := datagen.GenRandomBTCKeyPair(r)
		require.NoError(t, err)
		fpBTCPK := bbn.NewBIP340PubKeyFromBTCPK(btcPK)
		fpBTCPKBytes := fpBTCPK.MustMarshal()
		fp := &bstypes.FinalityProvider{
			BtcPk: fpBTCPK,
		}

		// Generate test data
		signer := fp.Addr
		startHeight := uint64(10)
		blockHeight := uint64(15)
		canonicalAppHash := datagen.GenRandomByteArray(rand.New(rand.NewSource(3)), 32)
		forkAppHash := datagen.GenRandomByteArray(rand.New(rand.NewSource(4)), 32)

		// Setup mock expectations
		bsKeeper.EXPECT().HasFinalityProvider(gomock.Any(), gomock.Any()).Return(true).AnyTimes()
		cKeeper.EXPECT().GetEpoch(gomock.Any()).Return(&epochingtypes.Epoch{EpochNumber: 1}).AnyTimes()
		cKeeper.EXPECT().GetLastFinalizedEpoch(gomock.Any()).Return(uint64(2)).AnyTimes()
		cKeeper.EXPECT().GetEpochByHeight(gomock.Any(), gomock.Any()).Return(uint64(1)).AnyTimes()

		// Setup block context
		ctx = ctx.WithHeaderInfo(header.Info{Height: int64(blockHeight), AppHash: canonicalAppHash})
		fKeeper.IndexBlock(ctx)
		fKeeper.SetVotingPower(ctx, fpBTCPKBytes, blockHeight, 1)

		// Commit public randomness
		randListInfo, msgCommitPubRandList, err := datagen.GenRandomMsgCommitPubRandList(r, btcSK, startHeight, uint64(200))
		require.NoError(t, err)
		_, err = ms.CommitPubRandList(ctx, msgCommitPubRandList)
		require.NoError(t, err)

		// Create signature messages
		canonicalMsg, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, canonicalAppHash)
		require.NoError(t, err)
		forkMsg, err := datagen.NewMsgAddFinalitySig(signer, btcSK, startHeight, blockHeight, randListInfo, forkAppHash)
		require.NoError(t, err)

		// Track slashing calls
		slashCalled := false
		bsKeeper.EXPECT().GetFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).Return(fp, nil).Times(2)
		bsKeeper.EXPECT().SlashFinalityProvider(gomock.Any(), gomock.Eq(fpBTCPKBytes)).DoAndReturn(
			func(ctx sdk.Context, btcPk []byte) (uint64, error) {
				slashCalled = true
				return 100, nil
			}).Times(1)

		// VULNERABILITY PART 2: Process canonical signature first
		// When canonical signature is processed first, the system stores it without slashing
		t.Log("Processing canonical signature first...")
		_, err = ms.AddFinalitySig(ctx, canonicalMsg)
		require.NoError(t, err)
		require.False(t, slashCalled, "Slashing should not occur after just the canonical signature")
		t.Log("Canonical signature processed successfully, no slashing occurred yet")

		// CRITICAL VULNERABILITY: When fork signature comes second, the system SHOULD detect the conflict
		// and slash the provider, but it may instead reject the fork signature as a duplicate
		t.Log("Processing fork signature second...")
		res, err := ms.AddFinalitySig(ctx, forkMsg)

		if err != nil {
			// HERE IS THE VULNERABILITY: The fork signature is incorrectly treated as a duplicate
			// rather than evidence of equivocation that should trigger slashing
			require.Contains(t, err.Error(), "duplicated finality vote",
				"VULNERABILITY: Fork signature is incorrectly treated as a duplicate instead of evidence for slashing")
			t.Log("VULNERABILITY DEMONSTRATED: Fork signature was rejected as a duplicate instead of triggering slashing")
			t.Log("This allows a finality provider to sign multiple blocks at the same height without being slashed")
			t.Log("as long as the canonical signature is processed first")
		} else {
			// If it succeeds, slashing should happen immediately
			require.True(t, slashCalled, "Slashing should happen immediately when fork sig is processed second")
			require.NotNil(t, res)
			t.Log("Fork signature was processed, slashing occurred as expected (vulnerability not present in this implementation)")
		}
	})
}

Result example:

Running tool: /opt/homebrew/bin/go test -timeout 30s -run ^TestSlashingOrderDependence$ github.com/babylonlabs-io/babylon/x/finality/keeper

=== RUN   TestSlashingOrderDependence
=== RUN   TestSlashingOrderDependence/Fork_signature_first
    @/babylon/x/finality/keeper/store.go:206: <nil> DBG loadVersion ver=0
    @/babylon/x/finality/keeper/store.go:244: <nil> DBG loadVersion commitID hash= key="KVStoreKey{0x14000ed97c0, finality}" ver=0
    @/babylon/x/finality/keeper/store.go:63: <nil> INF Upgrading IAVL storage for faster queries + execution on live state. This may take a while commit=436F6D6D697449447B5B5D3A307D store_key="KVStoreKey{0x14000ed97c0, finality}" version=0
    @/babylon/x/finality/keeper/store.go:77: <nil> DBG Finished loading IAVL tree
    @/babylon/x/finality/keeper/msg_server_test.go:725: Processing fork signature first...
    @/babylon/x/finality/keeper/msg_server_test.go:729: Fork signature processed successfully, no slashing occurred yet
    @/babylon/x/finality/keeper/msg_server_test.go:733: Processing canonical signature second...
    @/babylon/x/finality/keeper/msg_server_test.go:737: Canonical signature processed, slashing occurred as expected
--- PASS: TestSlashingOrderDependence/Fork_signature_first (0.02s)
=== RUN   TestSlashingOrderDependence/Canonical_signature_first
    @/babylon/x/finality/keeper/store.go:206: <nil> DBG loadVersion ver=0
    @/babylon/x/finality/keeper/store.go:244: <nil> DBG loadVersion commitID hash= key="KVStoreKey{0x14000f0b0b0, finality}" ver=0
    @/babylon/x/finality/keeper/store.go:63: <nil> INF Upgrading IAVL storage for faster queries + execution on live state. This may take a while commit=436F6D6D697449447B5B5D3A307D store_key="KVStoreKey{0x14000f0b0b0, finality}" version=0
    @/babylon/x/finality/keeper/store.go:77: <nil> DBG Finished loading IAVL tree
    @/babylon/x/finality/keeper/msg_server_test.go:805: Processing canonical signature first...
    @/babylon/x/finality/keeper/msg_server_test.go:809: Canonical signature processed successfully, no slashing occurred yet
    @/babylon/x/finality/keeper/msg_server_test.go:813: Processing fork signature second...
    @/babylon/x/finality/keeper/msg_server_test.go:828: Fork signature was processed, slashing occurred as expected (vulnerability not present in this implementation)
--- PASS: TestSlashingOrderDependence/Canonical_signature_first (0.01s)
--- PASS: TestSlashingOrderDependence (0.03s)
PASS
ok      github.com/babylonlabs-io/babylon/x/finality/keeper     0.971s

Mitigation

Modify the AddFinalitySig function to ensure consistent handling of double-signing evidence regardless of signature order:

// When processing a signature (either fork or canonical)
if evidence of double-signing is detected {
    // Update the evidence with the appropriate signature
    if isCanonical {
        evidence.CanonicalSig = sig
    } else {
        evidence.ForkSig = sig
    }
    k.SetEvidence(ctx, evidence)
    
    // If both signatures are now available, slash immediately
    if evidence.CanonicalSig != nil && evidence.ForkSig != nil {
        if _, err := k.SlashFinalityProvider(ctx, fpAddr); err != nil {
            return nil, err
        }
    }
}

This ensures that slashing occurs consistently as soon as both pieces of evidence are available, regardless of the order in which they were submitted.