Skip to content

Latest commit

 

History

History
86 lines (60 loc) · 2.76 KB

045.md

File metadata and controls

86 lines (60 loc) · 2.76 KB

Slow Misty Iguana

High

When Active stake btcDel.TotalSat should be divided by len(btcDel.FpBtcPkList)

Summary

When Active stake btcDel.TotalSat should be divided by len(btcDel.FpBtcPkList)

Root Cause

TotalSat is not divided by the number of FpBtcPkList.

Internal Pre-conditions

External Pre-conditions

Attack Path

Impact

Repeated TotalSat adds up, resulting in loss of funds or vote manipulation.

PoC

When CreateBTCDelegation, staker can specify multiple FPs:

func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCreateBTCDelegation) (*types.MsgCreateBTCDelegationResponse, error) {
	......
	for _, fpBTCPK := range parsedMsg.FinalityProviderKeys.PublicKeysBbnFormat {
		// get this finality provider
		fp, err := ms.GetFinalityProvider(ctx, fpBTCPK)
		if err != nil {
			return nil, err
		}
		// ensure the finality provider is not slashed
		if fp.IsSlashed() {
			return nil, types.ErrFpAlreadySlashed.Wrapf("finality key: %s", fpBTCPK.MarshalHex())
		}
	}
    .....
}

CreateBTCDelegation only verifies the validity of the transaction/signature/pre-signature and does not verify how many FPS TotalSat can stake, the staker can pass in any number of fpBTCPK.

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/btcstaking/keeper/msg_server.go#L198-L300

When processing ACTIVE messages, btcDel.TotalSat is added to activedSatsByFpBtcPk, the problem is that TotalSat is not divided by the length of fpBTCPK:

func (k Keeper) ProcessAllPowerDistUpdateEvents(
	ctx context.Context,
	dc *ftypes.VotingPowerDistCache,
	events []*types.EventPowerDistUpdate,
) *ftypes.VotingPowerDistCache {
	
    switch delEvent.NewState {
    case types.BTCDelegationStatus_ACTIVE:
        // newly active BTC delegation
        // add the BTC delegation to each restaked finality provider
        for _, fpBTCPK := range btcDel.FpBtcPkList {
            fpBTCPKHex := fpBTCPK.MarshalHex()
->            activedSatsByFpBtcPk[fpBTCPKHex] = append(activedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
        }

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

This will result in a fp.TotalBondedSat calculation error, and fp.TotalBondedSat will be added up repeatedly.

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

Mitigation

    for _, fpBTCPK := range btcDel.FpBtcPkList {
        fpBTCPKHex := fpBTCPK.MarshalHex()
-        activedSatsByFpBtcPk[fpBTCPKHex] = append(activedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
+        activedSatsByFpBtcPk[fpBTCPKHex] = append(activedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat/len(btcDel.FpBtcPkList))
    }