sherlock-audit / 2024-05-sophon-judging

1 stars 1 forks source link

zarkk01 - No slippage protection on convert functions in `SophonFarming`. #217

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

zarkk01

high

No slippage protection on convert functions in SophonFarming.

Summary

User can be front-runned on the deposit procedure and due to the conversion of token to lpToken lose instantly lot of value leading to less points and loss of funds for him.

Vulnerability Detail

Users can deposit their funds in the SophonFarming contract calling the relevant depositXXX funcion and convert their funds to the lpToken before the deposital. However, in this procedure there is no slippage protection that will guarantee that the user will got entitled to the same amount of lpToken as the amount of funds that he deposited and, also, the points that he deserved. For example, let's take the scenario that user calls depositStEth() function and wants to deposit 1 stETH. Then the _depositPredefinedAsset() function will be called that will try to convert the 1 stETH to a wstETH amount calling _stEthTOwstEth() wraping the stETH. However, nowhere the finalAmount returned is compared to be at least close to the initalAmount in seETH. That means that a malicious front-runner could perfom an attack on the WstETH part and make the finalAmount a lot less than expected by wrapping his big amount(maybe flash loaned) and then back running the user's transaction. This is totally possible, since the wrap function lies on the pooled ETH as we can see in the line 1076 of WstETH contract. This attack can happen on other depositXXX functions that convert the token to lpToken too but for simplicity we analyzed only this.

Impact

MEV Front-running attacks can be performed on the convert functions and diminish instantly the deposited amount of user and also the reward points that he is entitled to.

Code Snippet

All depositXXX functions : https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L796 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L808 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L821 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L832 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L843 https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L854

Tool used

Manual code inspection

Recommendation

Consider adding a slippage protection mechanism in convert functions in which the user will select the minimum amount of lpToken that he will accept of the conversion.

sherlock-admin3 commented 1 month ago

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

0xmystery commented:

invalid because users can always optionally call deposit() by pre-converting DAI/sDAI, ETH/stETH, ETH/eTH at their control