DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Invalid

Users can not unstake for current epoch with `unstakeAll`

Summary

The unstakeAll function skips deposits from the current epoch instead of unstaking them. Though it is possible to use another function to unstake such deposits some users can be faced with unexpected asset locking.

Vulnerability Details

Users can expect that the unstakeAll function will unstake even tokens which were deposited in the current epoch but the function just skip these deposits:

function unstakeAll()
external
checkEpochRollover
redeemPendingRewards
returns (uint256 totalStakedAmount)
{
uint256[] memory activeDeposits = getActiveDeposits(msg.sender);
if (activeDeposits.length == 0) revert NoActiveDeposit();
for (uint16 i = 0; i < activeDeposits.length; i++) {
uint16 epoch = uint16(activeDeposits[i]);
DepositReceipt storage dr = deposits[msg.sender][epoch];
>> if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;

This way such deposits can accidentally stay locked in the contract until lockCycle is passed.

Impact

Unexpected behavior, unexpected temporary asset locking

Tools used

Manual Review

Recommendations

Consider unstaking deposits from currentEpoch.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.