User could accidentally loose the tokens

Summary

User could accidentally deposit DAI into the contract via the depositEth function, and loose the assets due to the check missing.

Vulnerability Detail

User has 1 ETH and 1 DAI on the balance. User accidentally calls depositEth function and choose _predefinedPool as PredefinedPool.sDAI.

depositEth is called which transfer the ETH from the msg.sender account. This function doesn’t check if _predefinedPool == PredefinedPool.sDAI, so it proceeds further into the _depositPredefinedAsset function.
In the _depositPredefinedAsset function, there is a check if _predefinedPool == PredefinedPool.sDAI it transforms the DAI balance of the msg.sender into the sDAI via
```
 IsDAI(sDAI).deposit(_amount, address(this));
```
This function transferFrom msg.sender the DAI and mint sDAI.
After, the _deposit is called which deal with the Deposit an asset to SophonFarming logic.

Impact

Token lost. The user could double pay if accidentally call depositEth function with _predefinedPool as PredefinedPool.sDAI. The user could loose the ETH which will not be added into the contract. Also, the DAI could be deducted unintentionally

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/main/farming-contracts/contracts/farm/SophonFarming.sol#L503-L516

Tool used

Manual Review

Recommendation

Make the necessary check. Don’t allow to to deposit the DAI via the depositEth function.

sherlock-admin3 commented 1 month ago

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

0xmystery commented:

invalid because user input validation to prevent user mistakes is not considered a valid issue. Additionally, the likelihood of the scenario happening is extremely low

sherlock-admin2 commented 3 weeks ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/sophon-org/farming-contracts/commit/f0b82fd83a5d85eb8dc7ba2bbce2d49fefb326a4