code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

Users can send ZetaToken without paying out bound tx gas #240

Closed c4-bot-5 closed 10 months ago

c4-bot-5 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/gas_payment.go#L288

Vulnerability details

Impact

Users can send ZetaToken without paying out bound tx gas, cause the loss of the protocol funds.

Proof of Concept

When a user use ZetaToken send messages across the chain, in the module will use PayGasInZetaAndUpdateCctx to pay out bound tx gas, the function will deduct the amount of gasPrice * gasLimit out bound tx gas, and finally update the value of cctx.OutTxParam.Amount, the user will then receive the number of tokens after deducting gas and protocol fees.

The problem is that gasLimit is obtained from the user parameter, and if the user passes in a value of 0 gasLimit, there is no need to pay for Out Tx gas(or if the user passes in a small gasLimit, the user only pays a small Out Tx gas).

Other types of token gasLimit are fixed values obtained in the contract account via ChainGasParams, set by the administrator, and therefore do not have this problem.

When the observer processes Out Tx, the gasLimit value is less than 100_000 and is set to 100_000, so the transfer can be executed.

Let's take a look at the code:

gasLimit is obtained from the OutboundTxGasLimit parameter:

func (k Keeper) PayGasInZetaAndUpdateCctx(....){
    ....
    gasLimit := sdk.NewUint(cctx.GetCurrentOutTxParam().OutboundTxGasLimit)
}

destinationGasLimit is OutboundTxGasLimit:

    function send(ZetaInterfaces.SendInput calldata input) external override whenNotPaused {
        bool success = IERC20(zetaToken).transferFrom(msg.sender, address(this), input.zetaValueAndGas);
        if (!success) revert ZetaTransferError();
        emit ZetaSent(
            tx.origin,
            msg.sender,
            input.destinationChainId,
            input.destinationAddress,
            input.zetaValueAndGas,
@>          input.destinationGasLimit,
            input.message,
            input.zetaParams
        );
    }

Set OutboundTxGasLimit when creating the cctx:

func (k Keeper) CreateNewCCTX(....){
    ....
    outBoundParams := &types.OutboundTxParams{
        Receiver:                         msg.Receiver,
        ReceiverChainId:                  receiverChain.ChainId,
        OutboundTxHash:                   "",
        OutboundTxTssNonce:               0,
@>      OutboundTxGasLimit:               msg.GasLimit,
        OutboundTxGasPrice:               "",
        OutboundTxBallotIndex:            "",
        OutboundTxObservedExternalHeight: 0,
        CoinType:                         msg.CoinType, // FIXME: is this correct?
        Amount:                           sdk.NewUint(0),
        TssPubkey:                        tssPubkey,
    }
    ...
}

msg.GasLimit is parsed from the event:

func (ob *EVMChainClient) GetInboundVoteMsgForZetaSentEvent(...){
    ....
    return *GetInBoundVoteMessage(
        event.ZetaTxSenderAddress.Hex(),
        ob.chain.ChainId,
        event.SourceTxOriginAddress.Hex(),
        clienttypes.BytesToEthHex(event.DestinationAddress),
        destChain.ChainId,
        sdkmath.NewUintFromBigInt(event.ZetaValueAndGas),
        base64.StdEncoding.EncodeToString(event.Message),
        event.Raw.TxHash.Hex(),
        event.Raw.BlockNumber,
@>      event.DestinationGasLimit.Uint64(),
        common.CoinType_Zeta,
        "",
        ob.zetaClient.GetKeys().GetOperatorAddress().String(),
        event.Raw.Index, 
    ), nil
    ....
}

When the observer processes out tx, the gasLimit value is less than 100_000 and is set to 100_000:

func (signer *EVMSigner) TryProcessOutTx(...){
    ....
    gasLimit := send.GetCurrentOutTxParam().OutboundTxGasLimit
    if gasLimit < 100_000 {
        gasLimit = 100_000
        logger.Warn().Msgf("gasLimit %d is too low; set to %d", send.GetCurrentOutTxParam().OutboundTxGasLimit, gasLimit)
    }
    if gasLimit > 1_000_000 {
        gasLimit = 1_000_000
        logger.Warn().Msgf("gasLimit %d is too high; set to %d", send.GetCurrentOutTxParam().OutboundTxGasLimit, gasLimit)
    }
    ....
}

Tools Used

vscode manual

Recommended Mitigation Steps

Set a minimum value that is used when the OutboundTxGasLimit is less than the minimum

Assessed type

Other

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as primary issue

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as sufficient quality report

c4-sponsor commented 11 months ago

lumtis (sponsor) confirmed

0xean commented 11 months ago

this could be viewed as a duplicate of #406 as if #406 is resolved so is this issue. Will mark as duplicate for now.

c4-judge commented 11 months ago

0xean marked the issue as duplicate of #406

c4-judge commented 10 months ago

0xean marked the issue as satisfactory

c4-judge commented 10 months ago

0xean marked the issue as not a duplicate

c4-judge commented 10 months ago

0xean marked the issue as duplicate of #401