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

0 stars 0 forks source link

Risk of Uncompensated Gas Fee Overpayment #230

Closed c4-bot-4 closed 11 months ago

c4-bot-4 commented 12 months ago

Lines of code

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

Vulnerability details

Impact

The lack of an excess gas fee refund mechanism can result in users losing funds due to overpayment of gas fees. This could lead to financial losses for users and impact the usability and trustworthiness of the cross-chain transaction system.

Proof of Concept

The issue is observed in the functions PayGasNativeAndUpdateCctx, PayGasInERC20AndUpdateCctx, and PayGasInZetaAndUpdateCctx within the provided contract code. These functions calculate gas fees, deduct them from the transaction amount, and update the outbound transaction parameters. However, there is no provision for handling excess gas fees and refunding them to users.

// Example snippet from PayGasNativeAndUpdateCctx function
func (k Keeper) PayGasNativeAndUpdateCctx(
    ctx sdk.Context,
    chainID int64,
    cctx *types.CrossChainTx,
    inputAmount math.Uint,
) error {
    // ... (code for gas fee calculation)

    // subtract the withdraw fee from the input amount
    if outTxGasFee.GT(inputAmount) {
        return cosmoserrors.Wrap(types.ErrNotEnoughGas, fmt.Sprintf("outTxGasFee(%s) more than available gas for tx (%s) | Identifiers : %s ",
            outTxGasFee,
            inputAmount,
            cctx.LogIdentifierForCCTX()),
        )
    }
    ctx.Logger().Info("Subtracting amount from inbound tx", "amount", inputAmount.String(), "fee", outTxGasFee.String())
    newAmount := inputAmount.Sub(outTxGasFee)

    // update cctx
    cctx.GetCurrentOutTxParam().Amount = newAmount
    // ... (additional code for updating outbound transaction parameters)
    return nil
}

The provided test results indicate that the test cases for the gas payment functions (PayGasNativeAndUpdateCctx) have passed successfully. However, these tests do not cover scenarios where excess gas fees could occur.

=== RUN   TestKeeper_PayGasNativeAndUpdateCctx
=== RUN   TestKeeper_PayGasNativeAndUpdateCctx/can_pay_gas_in_native_gas
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx/can_pay_gas_in_native_gas (0.04s)
=== RUN   TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_not_coin_type_gas
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_not_coin_type_gas (0.00s)
=== RUN   TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_chain_is_not_supported
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_chain_is_not_supported (0.00s)
=== RUN   TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_can't_query_the_gas_price
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_can't_query_the_gas_price (0.02s)
=== RUN   TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_not_enough_amount_for_the_fee
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx/should_fail_if_not_enough_amount_for_the_fee (0.02s)
--- PASS: TestKeeper_PayGasNativeAndUpdateCctx (0.09s)
PASS
ok      github.com/zeta-chain/zetacore/x/crosschain/keeper      2.526s

Tools Used

Manual

Recommended Mitigation Steps

Implement a mechanism to refund excess gas fees to users.

Assessed type

Other

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 11 months ago

0xean marked the issue as unsatisfactory: Overinflated severity