039.md

Slow Misty Iguana

High

Incorrect handling of unvoted FPs in the HandleResumeFinalityProposal() function

Summary

A certain FP only did not vote in one specific block, but was penalized during the interval [haltingHeight, currentHeight + JailDuration], without considering the cases where this FP has already voted in those blocks.

Root Cause

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/finality/keeper/gov.go#L72

func (k Keeper) HandleResumeFinalityProposal(ctx sdk.Context, fpPksHex []string, haltingHeight uint32) error {
	// a valid proposal should be
	// 1. the halting height along with some parameterized future heights should be indeed non-finalized
	// 2. all the fps from the proposal should have missed the vote for the halting height
	// TODO introduce a parameter to define the finality has been halting for at least some heights

	params := k.GetParams(ctx)
	currentHeight := ctx.HeaderInfo().Height
	currentTime := ctx.HeaderInfo().Time

	// jail the given finality providers
	fpPks := make([]*bbntypes.BIP340PubKey, 0, len(fpPksHex))
	for _, fpPkHex := range fpPksHex {
		fpPk, err := bbntypes.NewBIP340PubKeyFromHex(fpPkHex)
		if err != nil {
			return fmt.Errorf("invalid finality provider public key %s: %w", fpPkHex, err)
		}
		fpPks = append(fpPks, fpPk)

		voters := k.GetVoters(ctx, uint64(haltingHeight))
		_, voted := voters[fpPkHex]
		if voted {
			// all the given finality providers should not have voted for the halting height
			return fmt.Errorf("the finality provider %s has voted for height %d", fpPkHex, haltingHeight)
		}

		err = k.jailSluggishFinalityProvider(ctx, fpPk)
		if err != nil && !errors.Is(err, bstypes.ErrFpAlreadyJailed) {
			return fmt.Errorf("failed to jail the finality provider %s: %w", fpPkHex, err)
		}

		// update signing info
		signInfo, err := k.FinalityProviderSigningTracker.Get(ctx, fpPk.MustMarshal())
		if err != nil {
			return fmt.Errorf("the signing info of finality provider %s is not created: %w", fpPkHex, err)
		}
@>		signInfo.JailedUntil = currentTime.Add(params.JailDuration)
		signInfo.MissedBlocksCounter = 0
		if err := k.DeleteMissedBlockBitmap(ctx, fpPk); err != nil {
			return fmt.Errorf("failed to remove the missed block bit map for finality provider %s: %w", fpPkHex, err)
		}
		err = k.FinalityProviderSigningTracker.Set(ctx, fpPk.MustMarshal(), signInfo)
		if err != nil {
			return fmt.Errorf("failed to set the signing info for finality provider %s: %w", fpPkHex, err)
		}

		k.Logger(ctx).Info(
			"finality provider is jailed in the proposal",
			"height", haltingHeight,
			"public_key", fpPkHex,
		)
	}

	// set the all the given finality providers voting power to 0
@>	for h := uint64(haltingHeight); h <= uint64(currentHeight); h++ {
		distCache := k.GetVotingPowerDistCache(ctx, h)
		activeFps := distCache.GetActiveFinalityProviderSet()
		for _, fpToJail := range fpPks {
			if fp, exists := activeFps[fpToJail.MarshalHex()]; exists {
				fp.IsJailed = true
				k.SetVotingPower(ctx, fpToJail.MustMarshal(), h, 0)
			}
		}

		distCache.ApplyActiveFinalityProviders(params.MaxActiveFinalityProviders)

		// set the voting power distribution cache of the current height
		k.SetVotingPowerDistCache(ctx, h, distCache)
	}

	k.TallyBlocks(ctx)

	return nil
}

As can be seen from the above code, a certain FP did not vote at haltingHeight, and its voter power from haltingHeight to currentHeight was set to 0. This is problematic because this FP might have voted in subsequent blocks such as haltingHeight + 1. It should receive the voting reward for haltingHeight + 1.

An honest FP is penalized in the same way as an FP that did not vote at all from [haltingHeight, currentHeight] just because it did not vote at haltingHeight. This is unfair.

Internal pre-conditions

External pre-conditions

The chain halted at the haltingHeight block.

Attack Path

Impact

This has led to the FP being treated unfairly and losing the bonus it should have received.

PoC

Assume the chain halted at haltingHeight = 3, with the current currentHeight being 5.

A certain FP (fp1) did not vote at height 3 but participated in voting at heights 4 and 5.

1. The fpList submitted by the governance (gov) includes fp1.
2. At height = 3, setting fp1's IsJailed to true and its VotingPower to 0 is correct.
3. However, at height = 4, it should not set fp1's IsJailed to true nor set its VotingPower to 0.

Mitigation

Retrieve Voters: For each height h, retrieve the list of voters using k.GetVoters(ctx, h). Compare FP with Voters: Check if the FP (fp) is present in the voters list. If FP is a Voter: Ensure that its VotingPower is not set to 0 for that height. If FP is Not a Voter: Proceed to set VotingPower to 0 as usual.

for h := uint64(haltingHeight); h <= uint64(currentHeight); h++ {
+    voters := k.GetVoters(ctx, h)
    // Compare FP with each voter in voters
+    if isVoter(fp, voters) {
+        // Do not set VotingPower to 0
    }
}