Skip to content

Latest commit

 

History

History
92 lines (63 loc) · 3.55 KB

048.md

File metadata and controls

92 lines (63 loc) · 3.55 KB

Active Wintergreen Porpoise

High

no check the input amt is negtive or not will lead to cause loss of funds for delegators

Summary

The missing amt negtive check in subDelegationSat will cause loss of funds for delegators as an attacker can manipulate the total active stake amount through integer overflow.

Root Cause

In https://github.com/babylonlabs-io/babylon/blob/c3aaaa1c5fef70bb32b3d37b2284b0284b4d28e2/x/incentive/keeper/reward_tracker_store.go#L162 there is a missing negtive check for amt when subtracting delegation amounts.

Internal Pre-conditions

  • A delegation needs to exist with a positive TotalActiveSat amount

  • The amt parameter needs to be negative, causing an integer overflow when subtracted

External Pre-conditions

None

Attack Path

1.Create a BTCDelegationStatus_UNBONDED event and with a BTCDelegation object such that the TotalSat field contains a very large value (close to the maximum value of uint64). This could involve exploiting a vulnerability in the delegation creation/update process. (the code below ,we can see sats is uint64)

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)
	})
}
  1. in the function BtcDelegationUnbonded will convert the sats from uint64 to sdkmath.Int. and if the sat is very big, which will lead to amtSat is negtive value.
func (k Keeper) BtcDelegationUnbonded(ctx context.Context, fp, del sdk.AccAddress, sat uint64) error {
	amtSat := sdkmath.NewIntFromUint64(sat)
	return k.btcDelegationModifiedWithPreInitDel(ctx, fp, del, func(ctx context.Context, fp, del sdk.AccAddress) error {
		return k.subDelegationSat(ctx, fp, del, amtSat)
	})
}

3.Inside subDelegationSat, the SubTotalActiveSat function is called. Because amtSat is so large, the subtraction btcDelRwdTracker.SubTotalActiveSat(amt) results in an integer overflow, wrapping around to a small positive number. 4.Incorrect Reward Calculation: The TotalActiveSat in the BTCDelegationRewardsTracker is now significantly lower than it should be. This will lead to incorrect reward calculations for the affected delegator and potentially for other delegators as well. 5.Reward Distribution Skewed: The attacker has effectively manipulated the reward distribution, potentially gaining an unfair share of the rewards at the expense of other delegators.

Impact

  • Loss of Funds: Legitimate delegators may receive fewer rewards than they are entitled to.
  • Reward Distribution Skew: The attacker gains an unfair advantage in the reward distribution.

PoC

func TestSubDelegationSat(t *testing.T) {
	k, ctx := NewKeeperWithCtx(t)

	fp1, del1 := datagen.GenRandomAddress(), datagen.GenRandomAddress()
	fp2, del2 := datagen.GenRandomAddress(), datagen.GenRandomAddress()
	amtFp1Del1 := math.NewInt(2000)

	_, err := k.GetBTCDelegationRewardsTracker(ctx, fp1, del1)
	require.EqualError(t, err, types.ErrBTCDelegationRewardsTrackerNotFound.Error())

	err = k.addDelegationSat(ctx, fp1, del1, amtFp1Del1)

	subAmtFp2Del2 := math.NewInt(350)

	err = k.subDelegationSat(ctx, fp2, del2, subAmtFp2Del2.Neg())

}

Mitigation

No response