scammed - DepositWrapper::deposit wstETH sent mistakenly will be stolen from the next user

[sherlock-admin4]

scammed

Medium

DepositWrapper::deposit wstETH sent mistakenly will be stolen from the next user

Summary

All mistakenly sent wstETH to DepositWrapper can be stolen from the subsequent depositor, and thus by depositing X amount of stETH he will get more lpTokens because the amounts will be higher.

Vulnerability Detail

When the user calls DepositWrapper::deposit, it will deposit X amount of stETH(say 1000 stETH). These tokens are then converted to wstETH inside _stethToWsteth, but when X amount is wrapped, the entire contract balance is stored inside amounts. Which will transfer more wstETH to the Vault compared to deposited stETH.

[DepositWrapper.sol#L38](https://github.com/sherlock-audit/2024-06-mellow/blob/26aa0445ec405a4ad637bddeeedec4efe1eba8d2/mellow-lrt/src/utils/DepositWrapper.sol#L38)

function _stethToWsteth(uint256 amount) private returns (uint256) {
      IERC20(steth).safeIncreaseAllowance(wsteth, amount);
      IWSteth(wsteth).wrap(amount);
      return IERC20(wsteth).balanceOf(address(this));
  }

This is not the case if the user deposits wstETH, the actual amount will be deposited there.

} else if (wsteth == token) {
  IERC20(wsteth).safeTransferFrom(sender, wrapper, amount);

Because of this inside Vault::deposit, he will receive more token in amounts, compared to the wstETH equivalent of deposited stETH, resulting in more lpTokens to be mined for less deposited stETH. If the user has manually wrapped these tokens and then called DepositWrapper::deposit with wstETH, this will not happen because in the wsteth case only the deposited amount is taken.

Impact

The user will receive free LPTokens for all mistakenly sent wstETH to DepositWrapper, exiting with free tokens and breaking the invariant of receiving lpTokens based on the initial deposit converted to wstETH.

Code Snippet

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

Tool used

Manual Review

The exact same issue was recently reported here - code-423n4/2024-05-loop-findings#33

Recommendation

Not IERC20(wsteth).balanceOf(address(this));, but the actual wrapped amount should be passed over.

[0x3b33]

Escalate

This issue is different from #299, it shows how more wstETH will be deposited because of current balance of a contract, please recheck it again.

[sherlock-admin3]

Escalate

This issue is different from #299, it shows how more wstETH will be deposited because of current balance of a contract, please recheck it again.

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.

[WangSecurity]

Agree with the escalation, I believe in this case sending wstETH directly into contract is the user mistake and not a valid H/M. If #299 is in the end valid (look into discussion under #181), this escalation will be accepted, deduplicated and invalidated.

[IlIlHunterlIlI]

@WangSecurity i think escalation is about questioining the validity of this issue and not to just dedup it, he is asking to dedup and validate
so escalations would be rejected either way, right?

[z3s]

I agree it's different from 299, but this issue is not M/H and already is non-reward invalid, so no change in rewards, according to rules this escalation should be rejected.

[WangSecurity]

Yep, agree with the comments above, planning to reject the escalation, dedup the issue, but leave invalid.

[WangSecurity]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0x3b33: rejected