Ironsidesec - Many cases stEth.transferFrom will transfer 1-2 less way, which would result in revert in consequent functions, because of not enough balance

[sherlock-admin2]

Ironsidesec

Medium

Many cases stEth.transferFrom will transfer 1-2 less way, which would result in revert in consequent functions, because of not enough balance

Summary

This issue is about 1 or 2 wei less transferred on steth.safeTransferfrom.

Also, there is another issue on steth.submit and wsteth.wrap which has a different impact and root cause than this.
This issue's root cause and fix are on steth.transferFrom and balance before/after checks.

Previous issues :

1. EgisSecurity - Many cases stEth::transferFrom will transfer 1-2 less way, which would result in revert in consequent functions, because of not enough balance 2024-05-sophon-judging#63
2. Account's stETH balance getting lower on 1 or 2 wei due to rounding down integer math lidofinance/core#442

Vulnerability Detail

The issue occurs in 2 functions with different impacts.

1. Issue on DepositWrapper.deposit, when converting from stETH to wstETH in L64-L65 below, if 1 eth is transferFrom called, it will pull 1eth - 1 or 2 wei less. And when the code block goes into _stethToWsteth which further calls wstETH.wrap which will revert due to less amount. That is, calling wrap(1 steth) but we have 1 steth - 1wei, so it reverts when wrapping. Look at https://github.com/lidofinance/lido-dao/blob/5fcedc6e9a9f3ec154e69cff47c2b9e25503a78a/contracts/0.6.12/WstETH.sol#L57

2. The second issue on Vault.deposit(), which calls stETH.transferfrom, which pulls 1 or 2 wei less amount which will deflate the share value on every deposit and messes the accounting. The current depositcallback doesn't care about the actualAmounts array which holds the 1 steth values instead of 1steth - 1 wei actual value. If deposit callbacks does any accounting on this actualAmounts, then the impact will increase.

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/utils/DepositWrapper.sol#L56

File: 2024-06-mellow/mellow-lrt/src/utils/DepositWrapper.sol

46:     function deposit() external payable returns (uint256 lpAmount) {
... SNIP ...

59:         if (token == steth) {
60:             IERC20(steth).safeTransferFrom(sender, wrapper, amount);
62:             amount = _stethToWsteth(amount);
63:         } else if (token == weth) {
64:   >>>>      IERC20(weth).safeTransferFrom(sender, wrapper, amount);
65:   >>>>      amount = _wethToWsteth(amount);
66:         } else if (token == address(0)) {

... SNIP ...

76:         (, lpAmount) = vault.deposit(to, amounts, minLpAmount, deadline);
80:     }

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L329-L332

File: 2024-06-mellow/mellow-lrt/src/Vault.sol

308:     function deposit()
... SNIP ...
318:     {
... SNIP ...

364:    >>>>     IERC20(tokens[i]).safeTransferFrom( 
365:                 msg.sender,
366:                 address(this),
367:                 amount
368:             );
371:         }

Impact

Reverts on DepositWrapper.deposit. so DOS most times. Also wrong accounting on Vault.deposit() messing the share value, i.e, mints 1 mellowToken for 1 wsteth - 2 wei deposit.

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/utils/DepositWrapper.sol#L56

https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/Vault.sol#L329-L332

Tool used

Manual Review

Recommendation

Use lido recommendation to utilize transferShares function, so the amount is realistic, or implement Fee on transfer approach, which compares the balance before and after the transfer.

Duplicate of #299