Skip to content

Latest commit

 

History

History
130 lines (95 loc) · 5.16 KB

030.md

File metadata and controls

130 lines (95 loc) · 5.16 KB

Slow Misty Iguana

Medium

The logic within the CreateBTCDelegation() function is flawed, resulting in the inclusion proof only being able to be added through the CreateBTCDelegation() interface

Summary

There is a logical error within the CreateBTCDelegation() function, which results in the system not supporting the Expression of Interest Delegation (EOI) mode.

Root Cause

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

func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCreateBTCDelegation) (*types.MsgCreateBTCDelegationResponse, error) {
	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), types.MetricsKeyCreateBTCDelegation)

	ctx := sdk.UnwrapSDKContext(goCtx)

	// 1. Parse the message into better domain format
	parsedMsg, err := types.ParseCreateDelegationMessage(req)

	if err != nil {
		return nil, status.Errorf(codes.InvalidArgument, "%v", err)
	}

	//skip

	// 5. Get params for the validated inclusion height either tip or inclusion height
	timeInfo, params, paramsVersion, err := ms.getTimeInfoAndParams(ctx, parsedMsg)
	if err != nil {
		return nil, err
	}
    //skip
}

The CreateBTCDelegation() function calls the ParseCreateDelegationMessage() function and the getTimeInfoAndParams() function.

// ParseCreateDelegationMessage parses a MsgCreateBTCDelegation message and performs some basic
// stateless checks:
// - unbonding transaction is a simple transfer
// - there is no duplicated keys in the finality provider key list
func ParseCreateDelegationMessage(msg *MsgCreateBTCDelegation) (*ParsedCreateDelegationMessage, error) {
	if msg == nil {
		return nil, fmt.Errorf("cannot parse nil MsgCreateBTCDelegation")
	}

	// NOTE: stakingTxProofOfInclusion could be nil as we allow msg.StakingTxInclusionProof to be nil
@>> 	stakingTxProofOfInclusion, err := NewParsedProofOfInclusion(msg.StakingTxInclusionProof)

//skip    
}

From ParseCreateDelegationMessage(), we understand that stakingTxProofOfInclusion could potentially be nil, meaning parsedMsg.StakingTxProofOfInclusion might be nil.

func (ms msgServer) getTimeInfoAndParams(
	ctx sdk.Context,
	parsedMsg *types.ParsedCreateDelegationMessage,
) (*DelegationTimeRangeInfo, *types.Params, uint32, error) {
	if parsedMsg.IsIncludedOnBTC() {
		// staking tx is already included on BTC
		// 1. Validate inclusion proof and retrieve inclusion height
		// 2. Get params for the validated inclusion height
		btccParams := ms.btccKeeper.GetParams(ctx)

@>		timeInfo, err := ms.VerifyInclusionProofAndGetHeight(
			ctx,
			btcutil.NewTx(parsedMsg.StakingTx.Transaction),
			btccParams.BtcConfirmationDepth,
			uint32(parsedMsg.StakingTime), //uint16 转32
			uint32(parsedMsg.UnbondingTime),
			parsedMsg.StakingTxProofOfInclusion,
		)

In the getTimeInfoAndParams() function, the VerifyInclusionProofAndGetHeight() function is called to verify parsedMsg.StakingTxProofOfInclusion.

func (k Keeper) VerifyInclusionProofAndGetHeight(
	ctx sdk.Context,
	stakingTx *btcutil.Tx,
	confirmationDepth uint32,
	stakingTime uint32,
	unbondingTime uint32,
	inclusionProof *types.ParsedProofOfInclusion,
) (*DelegationTimeRangeInfo, error) {
	// Check:
	// - timelock of staking tx
	// - staking tx is k-deep
	// - staking tx inclusion proof
@>	stakingTxHeader, err := k.btclcKeeper.GetHeaderByHash(ctx, inclusionProof.HeaderHash)
	if err != nil {
		return nil, fmt.Errorf("staking tx inclusion proof header %s is not found in BTC light client state: %v", inclusionProof.HeaderHash.MarshalHex(), err)
	}

However, the VerifyInclusionProofAndGetHeight() function does not handle the case where parsedMsg.StakingTxProofOfInclusion is nil, causing an error during its execution. As a result, when req.StakingTxInclusionProof is empty, calling CreateBTCDelegation() will fail and result in an error, leading to the failure of BTCDelegation creation.

Therefore, the system will only support the Single Step Delegation mode and will not support the Expression of Interest Delegation (EOI) mode.

This is inconsistent with the statement in the README. https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/btcstaking/README.md#single-step-delegation

Internal pre-conditions

External pre-conditions

The user wishes to create a BTC Delegation using the Expression of Interest Delegation (EOI) mode.

Attack Path

Impact

The AddBTCDelegationInclusionProof() function in msg_server.go has become useless. Users attempting to create a BTC Delegation using the Expression of Interest Delegation (EOI) mode will fail.

According to Sherlock's rules, the implemented functionality is inconsistent with the README's declaration, and the feature is incomplete. It should be rated as Medium.

PoC

Mitigation

Add handling for the case where arsedMsg.StakingTxProofOfInclusion is nil.