search

sherlock-audit / 2024-02-smilee-finance-judging

2 stars 1 forks source link

y0ng0p3 - Wrong calculation of tokenOutAmount - amountIn when swapping by _swapInSingle and_swapOutSingle #130

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 9 months ago

y0ng0p3

high

Wrong calculation of tokenOutAmount - amountIn when swapping by _swapInSingle and_swapOutSingle

Summary

Wrong calculation of tokenOutAmount - amountIn when swapping by UniswapAdapter::_swapInSingle and UniswapAdapter::_swapOutSingle functions.

Vulnerability Detail

Smilee is always using the _DEFAULT_FEE hardcoded to 500 to calculate the tokenOutAmount in UniswapAdapter::_swapInSingle() and amountIn in UniswapAdapter::_swapOutSingle().
There are several problems with the hardcoding of the 500 as the fee.

Specially as they are deploying in so many secondary chains like Kava, this will be a big problem pretty much in every transaction over there.

If any of those scenarios is given, tokenOutAmount/amountIn will be incorrectly calculated and it will return and incorrect amount of tokens swapped.

Impact

Incorrect tokenOutAmount/amountIn calculation leading to incorrect amount returned, either more or less tokens.

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L34 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L170-L187 https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/uniswap/UniswapAdapter.sol#L203-L221

Tool used

Manual review

Recommendation

Consider allowing the fees as an input.

sherlock-admin3 commented 8 months ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, UniswapAdapter allows to set arbitrary path (setPath), each segment in the path also specifies fee. If needed, admin can set such path from 1 segment with the needed fee. The usage of default fee seems to be intended functionality.

tsvetanovv commented:

Known issue. Check readme: "DEX SWAP: issues related to impacts of fees and slippage are known and mitigated"

takarez commented:

invalid