xKeywordx - [H-1] DepositWrapper::deposit function will revert because it will be off by 1-2 wei in the case of stETH deposits.

[sherlock-admin3]

xKeywordx

High

[H-1] DepositWrapper::deposit function will revert because it will be off by 1-2 wei in the case of stETH deposits.

Summary

According to Lido's official docs, there is a known issue called the 1-2 wei corner case. Internally, stETH transfers are converted to shares so if a user transfers 1e18 of stETH, the actual amount received by the protocol will be 0.99e18. You can see a description of the issue here.

Vulnerability Detail

The DepositWrapper::deposit function assumes that the amount parameter passed in by the function caller will be the actual amounts deposited in the protocol. In the case of stETH this will generally not be true because of the rounding that takes place during transfers.

Impact

The transaction will revert when the internal _stethToWsteth function will call the wrap function because the balance of the contract will be amount - 1 instead of amount.

When the function caller transfers stETH from him to the contract, the actual amount will be amount - 1, and when the DepositWrapper contract calls the wrap function, it will attempt to transfer amount from the contract to Lido, and it will revert because it has fewer tokens in it.

Code Snippet

    /// @inheritdoc IDepositWrapper
    function deposit(...) external payable returns (uint256 lpAmount) {
//..
//..
        if (token == steth) {
//@audit | the actual amount transferred here will be off by 1-2 wei
>           IERC20(steth).safeTransferFrom(sender, wrapper, amount);
>           amount = _stethToWsteth(amount);
        } else if (token == weth) {
//..
//..
    }

    function _stethToWsteth(uint256 amount) private returns (uint256) {
        IERC20(steth).safeIncreaseAllowance(wsteth, amount);
//@audit | the below line will revert because the actual amount held by the contract is `amount - 1`
>       IWSteth(wsteth).wrap(amount);
        return IERC20(wsteth).balanceOf(address(this));
    }

Tool used

Manual Review

Recommendation

Check the actual balance before and after stETH transfer in order to make sure that the protocol registers the right amount.

    /// @inheritdoc IDepositWrapper
    function deposit(...) external payable returns (uint256 lpAmount) {
//..
//..
        if (token == steth) {
+           uint256 balanceBefore = IERC20(stETH).balanceOf(address(this);
            IERC20(steth).safeTransferFrom(sender, wrapper, amount);
+           uint256 balanceAfter = IERC20(stETH).balanceOf(address(this);
+           uint256 actualAmount = balanceAfter - balanceBefore ;
-           amount = _stethToWsteth(amount);
+          amount = _stethToWsteth(actualAmount );
        } else if (token == weth) {
//..
//..
}

Duplicate of #299

[xKeywordx]

Escalate
This exact issue was credited as a High in the Sophon Farming contest on Sherlock. I don't understand why it is invalid in this contest. sherlock-audit/2024-05-sophon-judging#63

[sherlock-admin3]

Escalate
This exact issue was credited as a High in the Sophon Farming contest on Sherlock. I don't understand why it is invalid in this contest. sherlock-audit/2024-05-sophon-judging#63

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page,
in the Sherlock webapp.