code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/onboarding/keeper/ibc_callbacks.go#L93

Vulnerability details

Impact

It is not possible to select the maximum slippage when interacting with the AMM, through the middleware. There is a maximum amount that can be swapped and, since the middleware is meant to operate with very small amounts, the damages won't be great. That said: the middleware is supposed to provide users with 4canto (that at the time of writing would cost: 0.44$ or 0.000246eth) the swap limit for the 3 pools available at launch will be 10 usdc/usdt and 0.01eth, which mean that the users can be forced to pay up to, respectively, 22 times or 40 times more than the market price.

Proof of Concept

The swap module takes it's functionalities and code from IRISNET's Coinswap module. The function used in the middleware is: func (k Keeper) TradeInputForExactOutput(ctx sdk.Context, input types.Input, output types.Output) (sdk.Int, error) (https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/coinswap/keeper/swap.go#L163). In the context of the general AMM this function enables the user to specify the exact amount of tokens that they want to receive and the maximum amount of tokens that they are willing to spend. (as highlited by both comments and the error message in the function)

    // assert that the calculated amount is less than the
    // max amount the buyer is willing to pay.
    if soldTokenAmt.GT(input.Coin.Amount) {
        return sdk.ZeroInt(), sdkerrors.Wrap(types.ErrConstraintNotMet, fmt.Sprintf("insufficient amount of %s, user expected: %s, actual: %s", input.Coin.Denom, input.Coin.Amount.String(), soldTokenAmt.String()))
    }

The middleware passes the entire amount sent by the user as the input parameter, meaning that the function, that is intended to allow users to specify the maximum amount spendable, can spend as many as the tokens sent by the user as required to perform the swap.

    // parse the transferred denom
    transferredCoin := ibc.GetReceivedCoin(
        packet.SourcePort, packet.SourceChannel,
        packet.DestinationPort, packet.DestinationChannel,
        data.Denom, data.Amount,
    )

(https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/onboarding/keeper/ibc_callbacks.go#L81)

swappedAmount, err = k.coinswapKeeper.TradeInputForExactOutput(ctx, coinswaptypes.Input{Coin: transferredCoin, Address: recipient.String()}, coinswaptypes.Output{Coin: swapCoins, Address: recipient.String()})

(https://github.com/code-423n4/2023-06-canto/blob/a4ff2fd2e67e77e36528fad99f9d88149a5e8532/Canto/x/onboarding/keeper/ibc_callbacks.go#L93) Where transferedCoind (the amount of coins sent by the user to the IBC) will be used as the input amount coin of the TradeInputForExactOutput function.

There is only one limitation to the price of the trade: the TradeInputForExactOutput function has been customized to also include a maximum amount of tokens that can be spent by the user, per transaction:

if quoteCoinToSwap.Amount.GT(maxSwapAmount.Amount) {
        return sdk.ZeroInt(), sdkerrors.Wrap(types.ErrConstraintNotMet, fmt.Sprintf("expected swap amount %s%s exceeding swap amount limit %s%s", quoteCoinToSwap.Amount.String(), quoteCoinToSwap.Denom, maxSwapAmount.Amount.String(), maxSwapAmount.Denom))
    }

But, as per audit's documentation (https://github.com/code-423n4/2023-06-canto#swap), the maximum amount will be set to: "10 USDC, 10 USDT, 0.01 ETH". With the price of 4cantos being 0.44$ or 0.00246eth (at the time of writing), assuming the the underlying pool is perfectly balanced at the time when the order is placed, this open up the user to a slippage of up to, respectively, around: 2000%, 2000% and 4000% (which might be caused by other swaps or more realistically by large/unbalanced deposits). This is obviously the worst case scenario, but it highlights that the module, as is, has functionally no slippage control offered to the user.

Recommended Mitigation Steps

The module should offer the users the possibility to chose the amount of slippage desired or at least set a way smaller swap maximum amount.

Assessed type

Other

c4-pre-sort commented 1 year ago

JeffCX marked the issue as duplicate of #74

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-06-canto-findings