0xHabanero - LP Token amounts may be miscalculated due to DepositWrapper:: deposit() not handling stETH 1-2 wei corner case

[sherlock-admin4]

0xHabanero

High

LP Token amounts may be miscalculated due to DepositWrapper:: deposit() not handling stETH 1-2 wei corner case

Summary

As seen in the Lido official docs here, there is a very common edge case where sending steth (staked ETH) often results in 1-2 wei being left in the sender account during a transaction. This means that the true amount sent is slightly smaller than what is given as an argument in deposit. While the DepositWrapper:: deposit() attempts to handle any leftover wsteth by making sure that any remaining amount of wsteth in the contract is sent back to the sender, the main vulnerability lies in the way amount is calculated in DepositWrapper::deposit().

Vulnerability Detail

    function deposit(
        address to,
        address token,
        uint256 amount,
        uint256 minLpAmount,
        uint256 deadline
    ) external payable returns (uint256 lpAmount) {
       
        address wrapper = address(this);
        address sender = msg.sender;
        address[] memory tokens = vault.underlyingTokens();
        if (tokens.length != 1 || tokens[0] != wsteth)
            revert InvalidTokenList();
        if (amount == 0) revert InvalidAmount();
if (token == steth) {
            IERC20(steth).safeTransferFrom(sender, wrapper, amount);
            amount = _stethToWsteth(amount);

As can be seen in the code above, amount is being converted to wsteth after the transfer takes place, and is later being used for the vault deposit/LP token calculation. The issue with this is that in the likely 1-2 wei remainder case, the true amount being transferred is slightly smaller than what the argument amount equals. For example, if 1 eth is the amount to be deposited using DepositWrapper, amount will equal 1 eth. However, the true transfer amount is actually slightly less (.99e18 or a little less). As a result, the LP token amount will be calculated based on a 1 eth deposit, when in reality it is slightly less than 1 eth. This causes the user to receive LP tokens for 1 eth worth of deposit, while getting to keep their 1-2 wei that was "lost."

This shows how it is not only a vulnerability, but could be exploited by a user as a way to receive more LP Tokens than deserved, while still keeping the difference amount

Impact

Miscalculation of deposit token amount will inflate amount of LP tokens given to users who deposit stETH

Code Snippet

(https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/DepositWrapper.sol/?plain=1#L42)

Tool used

Manual Review

Recommendation

Since the initial balance of the wrapper will always be 0, something like the code below can give a more accurate calculation for the amount

if (token == steth) {
            IERC20(steth).safeTransferFrom(sender, wrapper, amount);
            amount = _stethToWsteth(IERC20(steth).balanceOf(wrapper));

As we can see, this calculates amount transferred using the balance of wrapper. This means even if 1-2 wei less is transferred due to this edge case, the LP token amount will be calculated using the exact and correct amount, further mitigating any potential exploitations or risks.

Alternatively, Lido's transferShares can be used to calculate amount more accurately.

[0xHabanero]

This bug causes a miscalculation of funds in the common 1-2 Wei corner case. As seen in past finding here, as well as here, this has been recognized as a valid, high finding in past contests. In this case, the situation is the exact same; the protocol has not correctly accounted for the 1-2 wei corner case, creating a protocol malfunction.

[sammy-tm]

Escalate

[sherlock-admin3]

Escalate

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[z3s]

Same as #181 I answered there.

[IlIlHunterlIlI]

@z3s totally wrong, txn will revert in wrap , not leading to any miscalculation

[WangSecurity]

The comment above is correct, the report is wrong and this issue won't lead to miscalculation of the LP token amounts, cause the deposit would just revert in this case. Hence, this issue is not correct and cannot be a sufficient duplicate cause it doesn't identify a valid medium impact.

Planning to reject the escalation and leave the issue as it is.

[WangSecurity]

Both comments are hidden due to being off-topic. The escalation is not resolved yet, and I'll take the necessary changes when the escalation will be resolved.

@0xHabanero be more respectful, the other Watson didn't say anything bad and we're professionals, so have to behave professionally.

[0xHabanero]

Understood @WangSecurity, apologies if I came off as disrespectful @IlIlHunterlIlI it was not my intention.

[0xHabanero]

For the finding itself @WangSecurity , I am not seeing how .wrap() would revert? Could you possibly tell me/show me the LOC where this would take place? Thank you!

[0xHabanero]

@WangSecurity Upon further review I see where this revert would take place and understand your decision to invalidate. Thanks.

[WangSecurity]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• sammy-tm: rejected