028.md

Slow Misty Iguana

High

Unfair reward distribution

Summary

HandleRewarding's loop skips unfinalized blocks, but the reward is not redistributed after the skipped block is Finalized.

Root Cause

The unfinalized block in theHandleRewarding function is skipped and no further processing is performed.

Internal Pre-conditions

1. Multiple blocks are voting at the same time.
2. The latter blocks are voted ahead of the previous blocks and enter the finalized state.

External Pre-conditions

Attack Path

1. Multiple blocks are voting at the same time, for example: h1, h2, h3.
2. h1 and h3 have been voted on(finalized), but h2 has not been voted on(unfinalized).
3. When HandleRewarding, h2 is skipped, nextHeightToReward is set to h3 + 1, h2 will not continue to distribute the proceeds, even if voted through.

Impact

In some blocks, fp cannot get reward, which is unfair reward calculation.

PoC

HandleRewarding is used to distribute reward, called when each block Endblock:

x/finality/abci.go/EndBlocker() -> HandleRewarding()

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/finality/keeper/rewarding.go#L33-L35

// HandleRewarding calls the reward to stakers if the block is finalized
func (k Keeper) HandleRewarding(ctx context.Context, targetHeight int64) {
        ......
	// targetHeight = heightToExamine = HeaderInfo().Height - k.GetParams(ctx).FinalitySigTimeout
	for height := nextHeightToReward; height <= uint64(targetHeight); height++ {
		block, err := k.GetBlock(ctx, height)
		if err != nil {
			panic(err)
		}
->		if !block.Finalized {
->			continue
		}
		k.rewardBTCStaking(ctx, height)
		nextHeightToReward = height + 1
	}

	if nextHeightToReward != copiedNextHeightToReward {
		k.SetNextHeightToReward(ctx, nextHeightToReward)
	}
}

The problem is that unfinalized blocks are skipped in this loop. After thecontinue statement,nextHeightToRewardwill be set after this block and the reward for this block will no longer be distributed.

That is likely to happen:

Because voting has a certain time period, the system allows fp to vote on multiple blocks at the same time.

Let's say there are three blocks voting: h1, h2, h3, their block Height is betweennextHeightToReward andheightToExamine(HeaderInfo().height-k.geetparams (ctx).finalitysigtimeout).

h1 and h3 have been voted and are Finalized, but h2 is not, so the loop skips h2 and setsnextHeightToReward to h3+1,

When h2 votes yes, since nextHeightToReward > h2,HandleRewarding started from h3+1, so h2 will never be able to distribute reward.

Mitigation

	for height := nextHeightToReward; height <= uint64(targetHeight); height++ {
		block, err := k.GetBlock(ctx, height)
		if err != nil {
			panic(err)
		}
		if !block.Finalized {
-			continue
+			break
		}
		k.rewardBTCStaking(ctx, height)
		nextHeightToReward = height + 1
	}