sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

Ironsidesec - `_canFinalizeWithdrawRequest` returns false on an edge case #83

Closed sherlock-admin4 closed 2 months ago

sherlock-admin4 commented 2 months ago

Ironsidesec

Medium

_canFinalizeWithdrawRequest returns false on an edge case

Summary

Unstake is possible on block.timestamp == userCooldown.cooldownEnd on sUSDe, but _canFinalizeWithdrawRequest returns false on that timestamp. So unstake/finalizing withdrawal is not possible due to wrong use of < instead of <=.

Vulnerability Detail

Look at _canFinalizeWithdrawRequest, it returns true if  userCooldown.cooldownEnd < block.timestamp. Bit it should return true even if userCooldown.cooldownEnd == block.timestamp. So use <= instead of  just <. If you look at unstake below, it allows to unstake even if block.timestamp >= userCooldown.cooldownEnd.

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/Ethena.sol#L177

 function _canFinalizeWithdrawRequest(uint256 requestId) internal view returns (bool) {
 uint24 duration = sUSDe.cooldownDuration();
 address holder = address(uint160(requestId));

 IsUSDe.UserCooldown memory userCooldown = sUSDe.cooldowns(holder);
 >>>   return (userCooldown.cooldownEnd < block.timestamp || 0 == duration);
 }

https://etherscan.io/address/0x9D39A5DE30e57443BfF2A8307A4256c8797A3497#code#F1#L84

 function unstake(address receiver) external {
 UserCooldown storage userCooldown = cooldowns[msg.sender];
 uint256 assets = userCooldown.underlyingAmount;
 >>>>   if (block.timestamp >= userCooldown.cooldownEnd || cooldownDuration == 0) {
 userCooldown.cooldownEnd = 0;
 userCooldown.underlyingAmount = 0;

 silo.withdraw(receiver, assets);
 } else {
 revert InvalidCooldown();
 }
 }

Impact

Unstake is possible on block.timestamp == userCooldown.cooldownEnd on sUSDe, but _canFinalizeWithdrawRequest returns false. So unstake/finalizing withdrawal is not possible due to the wrong use of < instead of <=.

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/Ethena.sol#L177

https://etherscan.io/address/0x9D39A5DE30e57443BfF2A8307A4256c8797A3497#code#F1#L84

Tool used

Manual Review

Recommendation

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/staking/protocols/Ethena.sol#L177

 function _canFinalizeWithdrawRequest(uint256 requestId) internal view returns (bool) {
 uint24 duration = sUSDe.cooldownDuration();
 address holder = address(uint160(requestId));

 IsUSDe.UserCooldown memory userCooldown = sUSDe.cooldowns(holder);
-       return (userCooldown.cooldownEnd < block.timestamp || 0 == duration);
+       return (userCooldown.cooldownEnd <= block.timestamp || 0 == duration);
 }
sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Trivial change