users funds can be directly lost in swap_exact_in() exchange

[hats-bug-reporter[bot]]

Github username: @skinneeomeje Twitter username: skinneeomeje Submission hash (on-chain): 0x1581144a5d9076c4bede6b48b799d3bdb143e11a3934576e845bc9da5f2d7a4e Severity: high

Description: Description\ https://github.com/Cardinal-Cryptography/common-amm-stable-swap/blob/60f7e5f1691d6de03323444cc3708933f7ba3117/amm/contracts/stable_pool/lib.rs#L800-L815 https://github.com/Cardinal-Cryptography/common-amm-stable-swap/blob/60f7e5f1691d6de03323444cc3708933f7ba3117/amm/contracts/stable_pool/lib.rs#L425-L482. https://github.com/Cardinal-Cryptography/common-amm-stable-swap/blob/60f7e5f1691d6de03323444cc3708933f7ba3117/amm/contracts/stable_pool/lib.rs#L454-L457

Attack Scenario\

The function swap_exact_in() is an exchange platform where users can directly swap any amount of asset represented in the code as token_in for any other allowed asset in this protocol as token_out with an expected min_token_out_amount which put a slippage control on the exchange in order to protect against excessive lost of funds while making trade. The swap_exact_in() function simply call a internal helper function _swap_exact_in(). Firstly, this function check the token id to see if the id of the tokens been transferred matches the expected token listed in the protocol. This then proceed to transferrring the token_in_id and the amount to the protocol through the function _transfer_in(). While this look good, the ensure!( token_out_amount >= min_token_out_amount, StablePoolError::InsufficientOutputAmount ); implements a check if swapped amount is not less than min_token_out_amount and if thus returns a StablePoolError::InsufficientOutputAmount error message which will be returned to the swap_exact_in() function. The purpose of this check is to ensure that the user gets out at least a certain amount of tokens but then if the check fail the amount of asset_in transfered to the protocol remains there with no means of retrieve this sent funds back!

Attachments

1. Proof of Concept (PoC) File

Now let's analyse this vulnerability

1. ADE wishes to exchange 1000 token_X for token_Y with min_token_out_amount of 995
2. A malicious actor frontruns this transaction in order to devalue token_X by selling a bunch of it for token token_Y
3. After that, ADE 1000 token_X is been transferred to the protocol as token_in_amount and the ensure!() check realise that the expected swapped amount is less than the min_token_out_amount returns a custom error message
4. This transcation succeeded in the blockchain with an error message and the state of the contract is not been reversed
5. ADE losses his 1000 token_X without receiving anything thing in return

Mititgation

Ensure check passes first before transferring user funds to protocol.

[JanKuczma]

Thank you for your submission.

When swap_exact_in(...) returns an error, all the state changes are reverted, including the transfer_from(...) call inside the _transfer_in(...) function.