Skip to content

Latest commit

 

History

History
88 lines (62 loc) · 3.33 KB

044.md

File metadata and controls

88 lines (62 loc) · 3.33 KB

Slow Misty Iguana

High

If fp is not in the VotingPowerDistCache, unbondedSats cannot be removed.

Summary

If fp is not in the VotingPowerDistCache, unbondedSats cannot be removed.

Root Cause

fp exists in unbondedSatsByFpBtcPk, but not in dc.FinalityProviders(VotingPowerDistCache).

Internal Pre-conditions

  1. The staker runs the unstake operation. The BTCDelegationStatus_UNBONDED event is sent and processed.
  2. The fp corresponding to unstake is not in the VotingPowerDistCache.

External Pre-conditions

Attack Path

  1. Malicious fp(attacker) increases their stake amount
  2. The attacker reduces the amount staked or gets jailed, not in the ActiveFps list (not in the VotingPowerDistCache).
  3. The attacker unstake(), and fp.TotalBondedSat did not decrease.
  4. The attacker restake to increase TotalBondedSat.
  5. By repeating the operation, the attacker can add any number of TotalBondedSat

Impact

Steal funds or vote manipulation

PoC

When processing UNBONDED events, the ProcessAllPowerDistUpdateEvents function now puts the required unbonded quantity into the unbondedSatsByFpBtcPk array, using the address of fp as the key:

func (k Keeper) processPowerDistUpdateEventUnbond(
	ctx context.Context,
	cacheFpByBtcPkHex map[string]*types.FinalityProvider,
	btcDel *types.BTCDelegation,
	unbondedSatsByFpBtcPk map[string][]uint64,
) {
	for _, fpBTCPK := range btcDel.FpBtcPkList {
		fpBTCPKHex := fpBTCPK.MarshalHex()
->		unbondedSatsByFpBtcPk[fpBTCPKHex] = append(unbondedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
	}
	k.processRewardTracker(ctx, cacheFpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
		k.MustProcessBtcDelegationUnbonded(ctx, fp, del, sats)
	})
}

Then iterate through the unbondedSatsByFpBtcPk array and reduce fp.TotalBondedSat:

		if fpUnbondedSats, ok := unbondedSatsByFpBtcPk[fpBTCPKHex]; ok {
			// handle unbonded delegations for this finality provider
			for _, unbodedSats := range fpUnbondedSats {
->				// RemoveBondedSats -> v.TotalBondedSat -= sats
->				fp.RemoveBondedSats(unbodedSats)
			}
			// remove the finality provider entry in fpUnbondedSats map, so that
			// after the for loop the rest entries in fpUnbondedSats belongs to new
			// finality providers that might have btc delegations entries
			// that activated and unbonded in the same slice of events
			delete(unbondedSatsByFpBtcPk, fpBTCPKHex)
		}

But the problem is that the function does not traverse unbondedSatsByFpBtcPk separately, but traverses it again when traversing dc.FinalityProviders and fpActiveBtcPkHexList:

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/finality/keeper/power_dist_change.go#L264-L369

If the key of fp exists in unbondedSatsByFpBtcPk, but does not exist in dc.FinalityProviders and fpActiveBtcPkHexList, the fp will not be processed and fp.TotalBondedSat will not be reduced.

fpActiveBtcPkHexList will only add content when there is an Active event.

dc.FinalityProviders is the currently active ActiveFps, dc := k.GetVotingPowerDistCache(ctx, height-1)

Therefore, all keys of fp may not exist in these two arrays.

TotalBondedSat is used to calculate rewards and voting rights, which can lead to loss of funds or vote manipulation.

Mitigation

Traverse unbondedSatsByFpBtcPk individually like activedSatsByFpBtcPk