sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

Koolex - Trader could possibly lose funds (i.e. pay more quoteToken) #78

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Koolex

medium

Trader could possibly lose funds (i.e. pay more quoteToken)

Summary

The trader has no way to set a maximum inputAmount. If for example the price returned by the Oracle (any Oracle) is wrong or inaccurate which is possible, the trader will pay more quoteToken than what they expect.

Vulnerability Detail

_getQuoteAmount method gets unscaledAmountToBeneficiary from Uniswap oracle.

    uint256 unscaledAmountToBeneficiary = OracleLibrary.getQuoteAtTick({
        tick: arithmeticMeanTick,
        baseAmount: quoteParams_.baseAmount,
        baseToken: cqp.cBase,
        quoteToken: cqp.cQuote
    });

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-oracle/src/UniV3OracleImpl.sol#L277-L282

It is possible that the price is stale or wrong for and as there is no maximum inputAmount check engorced as a safety, Trader could possibly pay more quoteToken due to the lack of the safety check. i.e. Swapper owner will receive more quote tokens and the trader will receive less which is a loss to the trader.

Note: this is applicable to any other oracle and not limited to Uniswap oracle.

Impact

Trader will receive less tokens and the Swapper owner will receive more. In other words, Trader will have a loss of funds.

Code Snippet

Check above

Tool used

Manual Review

Recommendation

Add a parameter maxInputAmount to flash method to allow the trader to set a max input amount (i.e. max quote amount) to be transferred from him/her. Then If amountsToBeneficiary is greater than maxInputAmount, revert.

Duplicate of #47