Audinarey - DepositWrapper::deposit(...) will revert for stETH

[sherlock-admin4]

Audinarey

High

DepositWrapper::deposit(...) will revert for stETH

Summary

User calls DepositWrapper::deposit(...) to deposit specified tokens into the vault, converting them to the required format if necessary. If the user specifies stETh as the token to deposit, the stETh is transfered into the DepositWrapper and the _stethToWsteth(...) is called to convert the stETH into wstETH

File: DepositWrapper.sol

35:     function _stethToWsteth(uint256 amount) private returns (uint256) {
36:         IERC20(steth).safeIncreaseAllowance(wsteth, amount); 
37: @>      IWSteth(wsteth).wrap(amount); 
38:         return IERC20(wsteth).balanceOf(address(this));
39:     }

42:     function deposit(
43:         address to,
44:         address token,
45:         uint256 amount,
46:         uint256 minLpAmount,
47:         uint256 deadline
48:     ) external payable returns (uint256 lpAmount) {
SNIP
....
55:         if (token == steth) { 
56: @>          IERC20(steth).safeTransferFrom(sender, wrapper, amount); 
57: @>          amount = _stethToWsteth(amount); // this returns shares 

The problem is that the protocol always assumes that the amount of stETh tokens received is equal to the amount of tokens transferred into the DepositWrapper.
This is not the case for rebasing tokens, such as stETH, because internally it transfers shares which generally results in the received amount of tokens being lower than the requested one by 1-2 wei because of roundings.
For instance transferring 1e18 eETH tokens from Alice to DepositWrapper, will result in DepositWrapper receiving 0.99...e18 eETH tokens.

Lido acknowledges this issue here and it is discussed extensively [here](lido-dao/issues/442

Vulnerability Detail

• User calls DepositWrapper::deposit(...) specifying token = stETh and amount = 1 ether
• 1 ether is transfered to DepositWrapper on L56 but due to rounding issue DepositWrapper receives 1 ether. minus 1wei (1 ether - 1)
• the DepositWrapper now has less than 1 ether
• stethToWsteth(amount) is now called on L57 to wrap the exact 1 ether which will attempt to transfer 1 ether from the DepositWrapper
• The function reverts because DepositWrapper now has less than 1 ether to be transfered out

Impact

This breaks accounting as well as well as a denial of service

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/DepositWrapper.sol#L55-L57

Tool used

Manual Review

Recommendation

Modify the DepositWrapper::deposit(...) function as shown below

42:     function deposit(
43:         address to,
44:         address token,
45:         uint256 amount,
46:         uint256 minLpAmount,
47:         uint256 deadline
48:     ) external payable returns (uint256 lpAmount) {
SNIP
....
55:         if (token == steth) { 
56:             IERC20(steth).safeTransferFrom(sender, wrapper, amount); 

57:  -          amount = _stethToWsteth(amount); 
57:  +          amount = _stethToWsteth(IERC20(steth).balanceOf(address(this)); 

Duplicate of #299