Users can lose their LP tokens after several withdrawal requests before the first rebalance.

Summary

MainVault.withdrawalRequest can be called several times when rebalancingPeriod = 0 so users can lose their LP tokens when they request withdrawal several times.

Vulnerability Detail

MainVault.withdrawalRequest can be called several times when rebalancingPeriod = 0. withdrawalRequest validates user.withdrawalRequestPeriod = 0 first and then sets user.withdrawalRequestPeriod = rebalancingPeriod.

  function withdrawalRequest(
    uint256 _amount
  ) external nonReentrant onlyWhenVaultIsOn returns (uint256 value) { 
    UserInfo storage user = userInfo[msg.sender];
    require(user.withdrawalRequestPeriod == 0, "Already a request");

    value = (_amount * exchangeRate) / (10 ** decimals());

    _burn(msg.sender, _amount);

    user.withdrawalAllowance = value;
    user.withdrawalRequestPeriod = rebalancingPeriod;
    totalWithdrawalRequests += value;
  }

So when rebalancingPeriod > 0, withdrawalRequest can't be called multiple times. But when rebalancingPeriod = 0 before the first rebalance, withdrawalRequest can be called several times without reversion.

If a user request withdrawal several times by fault, the LP tokens are burned multiple times, but the user’s withdrawalAllowance will be only the last one. So the user can lose his LP tokens.

Impact

The users can lose their LP tokens.

Code Snippet

https://github.com/sherlock-audit/2023-01-derby/blob/main/derby-yield-optimiser/contracts/MainVault.sol#L149-L162

Tool used

Manual Review

Recommendation

Use another flag withdrawalRequested in UserInfo struct to block multiple withdrawal request.

Duplicate of #392

sherlock-audit / 2023-01-derby-judging

XKET - Users can lose their LP tokens after several withdrawal requests before the first rebalance. #381