0xboriskataa - User can deposit into sDAI pool using ETH

[sherlock-admin2]

0xboriskataa

medium

User can deposit into sDAI pool using ETH

Summary

User can deposit into sDAI pool using ETH

Vulnerability Detail

In SophonFarming.sol there is a function depositEth in which a user can specify a predefined pool and deposit ETH into it:

    function depositEth(uint256 _boostAmount, PredefinedPool _predefinedPool) public payable {
        if (msg.value == 0) {
            revert NoEthSent();
        }

        uint256 _finalAmount = msg.value;
        if (_predefinedPool == PredefinedPool.wstETH) {
            _finalAmount = _ethTOstEth(_finalAmount);
        } else if (_predefinedPool == PredefinedPool.weETH) {
            _finalAmount = _ethTOeEth(_finalAmount);
        }

        _depositPredefinedAsset(_finalAmount, msg.value, _boostAmount, _predefinedPool);
    }

As you can see from the code snippet it checks if _predefinedPool is either wstETH or weETH and converts the user's ETH into the coresponding asset and deposits that asset. However there is one more asset that exists in the PredefinedPool struct which is sDAI.

If a user specifies sDAI as a _predefinedPool the function will not revert. The code will continue its executon into _depositPredefinedAsset where it will convert some of the DAI the contract holds into sDAI and then update the pool's balance.

    function _depositPredefinedAsset(uint256 _amount, uint256 _initalAmount, uint256 _boostAmount, PredefinedPool _predefinedPool) internal {

        uint256 _finalAmount;

        if (_predefinedPool == PredefinedPool.sDAI) {
            _finalAmount = _daiTOsDai(_amount);
        } else if (_predefinedPool == PredefinedPool.wstETH) {
            _finalAmount = _stEthTOwstEth(_amount);
        } else if (_predefinedPool == PredefinedPool.weETH) {
            _finalAmount = _eethTOweEth(_amount);
        } else {
            revert InvalidDeposit();
        }

        // adjust boostAmount for the new asset
        _boostAmount = _boostAmount * _finalAmount / _initalAmount;

        _deposit(typeToId[_predefinedPool], _finalAmount, _boostAmount);
    }

Impact

Wrong conversions happen. An amount of DAI corresponding to the amount of ETH the user deposits gets converted into sDAI even though the user deposited ETH not DAI.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add an else statement in which the function reverts:

    function depositEth(uint256 _boostAmount, PredefinedPool _predefinedPool) public payable {
        if (msg.value == 0) {
            revert NoEthSent();
        }

        uint256 _finalAmount = msg.value;
        if (_predefinedPool == PredefinedPool.wstETH) {
            _finalAmount = _ethTOstEth(_finalAmount);
        } else if (_predefinedPool == PredefinedPool.weETH) {
            _finalAmount = _ethTOeEth(_finalAmount);
        }
+      else {
+           revert InvalidPool();
+      }

        _depositPredefinedAsset(_finalAmount, msg.value, _boostAmount, _predefinedPool);
    }

[sherlock-admin2]

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]

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

[sherlock-admin2]

The Lead Senior Watson signed off on the fix.