sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

0xkmg - No slippage protection for `_ethTOstEth` #165

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

0xkmg

medium

No slippage protection for _ethTOstEth

Summary

_ethTOstEth contains no slippage protection, which makes it vulnerable to sandwich attacks, MEV exploits and may lead to significant loss

Vulnerability Detail

When users deposit through depositEth()and choose stETH as the predefined pool, the internal function _ethTOstEth is called to swap ETH to stETH. However, this function does not include any slippage protection or deadline constraints. This means that if the price of stETH fluctuates significantly between the time the transaction is submitted and when it is executed, users could receive much less stETH than expected. Additionally, the absence of these safeguards makes the function vulnerable to sandwich attacks and MEV exploits, where attackers manipulate transaction order to profit at the expense of users.

Impact

The lack of slippage protection in the _ethTOstEth function can lead to:

Significant Financial Losses: Users may receive fewer stETH tokens than expected during periods of high volatility. Sandwich Attacks and MEV Exploits: Attackers can manipulate transaction ordering to extract value from users, leading to higher costs and reduced returns.

Code Snippet

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

Tool used

Manual Review

Recommendation

Recommendation To mitigate the risk of slippage and MEV exploits, it is recommended to implement slippage protection and deadline constraints in the _ethTOstEth function. Here’s how you can enhance the function:

Implement Slippage Protection: Allow users to specify a minimum acceptable amount of stETH they are willing to receive for their ETH. Add Deadline Parameter: Include a timestamp after which the transaction will revert if not yet executed, preventing it from being delayed and executed under unfavorable conditions.


/**
 * @notice Converts ETH to stETH with slippage protection
 * @dev Lido
 * @param _amount in amount
 * @param _minOut minimum amount of stETH to receive
 * @param _deadline transaction deadline
 * @return uint256 out amount
 */
function _ethTOstEth(uint256 _amount, uint256 _minOut) internal returns (uint256) {

    uint256 balanceBefore = IERC20(stETH).balanceOf(address(this));
    IstETH(stETH).submit{ value: _amount }(address(this));
    uint256 amountReceived = IERC20(stETH).balanceOf(address(this)) - balanceBefore;

    require(amountReceived >= _minOut, "Slippage protection: insufficient output amount");
    return amountReceived;
}
sherlock-admin2 commented 5 months 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