Closed sherlock-admin4 closed 4 months ago
Escalate the loss of 1-2 WEI is not a low/invalid issue, the same issue has been considered in past audits as well thank you
Escalate the loss of 1-2 WEI is not a low/invalid issue, the same issue has been considered in past audits as well thank you
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.
I believe #181 is the duplicate of #299 that was escalated the earliest, so it will be the one to decide if this family is valid. Hence, this escalation is about the same and doesn't bring new information. Planning to reject it, cause regardless of it being accepted/rejected, the earlier escalation on #181 will give the same outcome.
After looking deeper into #181, I see it's not a sufficient duplicate, hence, the escalation there will be rejected either way. Hence, I'll make this escalation as the main one for #299 issue family.
Plan to accept the escalation and validate with high seveirty.
Plan to accept the escalation and validate with high severity.
I think this family should be Medium. because there is no fund loss, and only stETH deposits get DoSed most of time. users can still convert stETH to wstETH themselves.
Fair point, but I believe this issue qualifies for high severity:
Inflicts serious non-material losses (doesn't include contract simply not working).
In this case, major part of the deposits will be DOSed and since the protocol specifically allows to deposit stETH and will convert it for the user, I believe this is a serious non-material loss.
The decision remains the same, planning to accept the escalation and validate with high severity.
The decision remains the same, planning to reject the escalation and leave the issue as it is.
I think you mean accept the escalation.
Fair point, but I believe this issue qualifies for high severity:
Inflicts serious non-material losses (doesn't include contract simply not working).
In this case, major part of the deposits will be DOSed and since the protocol specifically allows to deposit stETH and will convert it for the user, I believe this is a serious non-material loss.
The decision remains the same, planning to accept the escalation and validate with high severity.
This issue is invalid as I have explained in detail in the comment here https://github.com/sherlock-audit/2024-06-mellow-judging/issues/205#issuecomment-2237222416
Wrapping will work with shares of the amount which remains the same for an amount
of steth tokens unless the shares/underlying balance in lido changes. Please attach a POC if otherwise, I am attaching one proving my point
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import {Test, console} from "forge-std/Test.sol";
interface IStETH {
function transfer(address to,uint amount) external returns(bool) ;
function submit(address referral) external payable returns(uint) ;
}
interface IWeth {
function withdraw(uint amount) external ;
function transfer(address to,uint amount) external returns(bool) ;
}
interface IWSteth {
function wrap(uint amount) external returns(uint) ;
}
interface IERC20 {
function approve(address to,uint amount) external returns(bool) ;
}
contract stETHTransferTest is Test {
address stETH = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;
address wsteth = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0;
address weth = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
function testWethToWstETH() public {
vm.createSelectFork(vm.envString("eth_rpc"));
vm.rollFork(20355828);
address wethWhale = 0x1681195C176239ac5E72d9aeBaCf5b2492E0C4ee;
uint amount = 3854154965781563458935;
vm.prank(wethWhale);
IWeth(weth).transfer(address(this),amount);
IWeth(weth).withdraw(amount);
IStETH(stETH).submit{value: amount}(address(0));
IERC20(stETH).approve(address(wsteth), amount);
IWSteth(wsteth).wrap(amount);
}
receive() external payable{
}
}
And further tweaking the POC, I've got the following two scenarios:
And none of the reverts at any point. In the end, regardless of which token you deposit (ETH, WETH, stETH or wstETH), each of them leads to calling _stethToWsteth
which returns the amount of wstETH inside the contract and deposits this amount into Vault. Hence, this issue wouldn't cause any reverts or any accounting issues. Planning to reject the escalation and leave the issue as it is.
Disregard my previous comment. I double-checked some things and it makes sense now why this doesn't happen.
Result: Invalid Duplicate of #299
0xBhumii
High
Potential Denial of Service (DoS) in
DepositWrapper
Contract Due to Rounding Errors instETH
TransfersSummary
The
deposit
function in theDepositWrapper
contract is susceptible to a known issue with thestETH
token where rounding errors can cause transferred shares to be 1-2 wei less than the intended_amount
. This discrepancy can lead to subsequent function calls failing due to insufficient balance, resulting in a potential denial of service (DoS) for users attempting to depositstETH
.Vulnerability Detail
The issue arises when the
deposit
function processesstETH
tokens. ThestETH
token uses shares for tracking balances, and due to rounding errors, the transferred shares may be 1-2 wei less than the_amount
passed. This can cause a mismatch in the expected amount ofstETH
transferred and wrapped intowstETH
, leading to failures in subsequent operations.Detailed Flow:
deposit
function receives anamount
parameter, which is passed toIERC20(steth).safeTransferFrom(sender, wrapper, amount)
._stethToWsteth
function is called to convertsteth
towsteth
._stethToWsteth
function may end up with 1-2 wei lessstETH
due to rounding issues.wstETH
, to fail because the actual balance is slightly less than expected.Impact
The probability of issue appearing is high and you can check in the following discussion. It has also been classified as a High severity on past contests: https://github.com/lidofinance/lido-dao/issues/442 The impact can be Contract DOS and incorrect balance handling
Code Snippet
https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/DepositWrapper.sol#L55C1-L58C1
Tool used
Manual Review
Recommendation
Use lido recommendation to utilize transferShares function, so the _amount is realistic, or implement FoT approach, which compares the balance before and after the transfer.
Duplicate of #299