Closed howlbot-integration[bot] closed 2 months ago
@devdks25 Sponsors are not allowed to close, reopen, or assign issues or pull requests.
Rebasing tokens aren't supported by the vaults. @dewpe
This issue is taken care of by the transferAmount = Math.min(totalAssets(), totalAssetsToSlash);
on L198 of the Vault.sol
because both the balanceOf
and transfer
functions of the stETH vanilla contract call getPooledEthByShares
underneath so they both would have the 1-2 gwei rounding issue. The Math.min
would then account for the 1-2 gwei underflow
Invalid.
Under the hood, stETH's transfer()
functions calculates the amount of shares to transfer with getSharesByPooledEth()
:
function getSharesByPooledEth(uint256 _ethAmount) public view returns (uint256) {
return _ethAmount
.mul(_getTotalShares())
.div(_getTotalPooledEther());
}
For the second transfer to revert, getSharesByPooledEth(amount)
in the second transfer has to be less than getSharesByPooledEth(amount)
in the first. However, as _ethAmount
, _getTotalShares()
and _getTotalPooledEther()
are the same for both transfers, the amount of shares transferred will be the same. It is not possible for the second transfer to revert.
If the warden believes otherwise, consider providing a PoC that forks mainnet and actually interacts with stETH.
MiloTruck marked the issue as unsatisfactory: Invalid
Lines of code
https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/SlashingHandler.sol#L56
Vulnerability details
Impact
Slashing stETH vaults may sometimes fail and always revert due to its innate 1-2 wei corner issue.
Proof of Concept
The protocol plans on working with all ERC20 tokens, and that includes stETH. stETH however, has intrinsic rounding down problems. It's a known issue, and the stETH balance of an account could be lower of 1-2 wei because of rounding down. As such the amount received during transfer can be short 1 or 2 wei. This isn't much, but in context of slashing, in which an amount is transferred from the vault is also expected to be the amount transferred to address(0), this can mean unexpected failure of the
handleSlashing
function, potentially dossing/delaying slashing of malicious operators.In SlashingHandler.sol, we see the
handleSlashing
function. The function transfers the amount of tokens to slash from the vault to the handler. After, the function attempts to transfer the "same" amount address(0).Final effect As explained that the amount received during transfer can be 1 or 2 wei shorter, the function's attempt to send the same amount fails and slashing is not completed.
Runnable POC
The gist link below contains modifications of some of the provided tests, including a mock token that simulates stETH losing 1 wei upon transfers, and instructions on how to run them.
https://gist.github.com/ZanyBonzy/033ee1ca8aae4969a676d8ce83a68c9d
The expected should look like this with an error that reads ERC20InsufficientBalance
Tools Used
Manual Review
Recommended Mitigation Steps
Recommend querying token balances before and after every transfer, and transferring the difference between them instead.
Assessed type
Token-Transfer