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

1 stars 0 forks source link

Doesn’t have proper slippage control. #32

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/swap.go#L194 https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/swap.go#L199 https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/swap.go#L210

Vulnerability details

Impact

For risk management purposes, a swap will fail if the input coin amount exceeds a predefined limit. But it is not a slippage control. It doesn’t consider how many Canto a user wants to swap for. It is possible that the user will swap the token at a very bad price.

Proof of Concept

In TradeInputForExactOutput, it only confirms that the input amount should be less than maxSwapAmount. https://github.com/code-423n4/2023-06-canto/blob/main/Canto/x/coinswap/keeper/swap.go#L194

func (k Keeper) TradeInputForExactOutput(ctx sdk.Context, input types.Input, output types.Output) (sdk.Int, error) {
    …

    maxSwapAmount, err := k.GetMaximumSwapAmount(ctx, quoteCoinToSwap.Denom)
    if err != nil {
        return sdk.ZeroInt(), err
    }

    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))
    }

    …
}

However, it doesn’t consider the output amount. The predefined max swap amount for USDC is 10 USDCs. Since it doesn’t consider the output amount of Canto. The user could pay 10 USDCs to get a little Cantos. In the onboarding process, the user could pay 10 USDCs to get 4 Cantos. And according to the actual price, that is an unfair swap for the user.

Tools Used

Manual Review

Recommended Mitigation Steps

Add an actual slippage control. Or the check on max swap amount should also consider the amount of output token.

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