withdrawalRequest() maybe lost funds in the first period

Summary

no limit cannot request when rebalancingPeriod == 0 , user maybe lost funds

Vulnerability Detail

withdrawalRequest() for withdrawal request for when the vault doesn't have enough funds available The code is as follows:

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

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

    _burn(msg.sender, _amount);

    user.withdrawalAllowance = value;
    user.withdrawalRequestPeriod = rebalancingPeriod; //<--------if rebalancingPeriod==0, can request again
    totalWithdrawalRequests += value;
  }

There is a problem here the period starts at 0 if the current is in the first period (rebalancingPeriod ==0), multiple requests will be overwritten by subsequent ones Example: assume rebalancingPeriod = 0

1.alice call withdrawalRequest(_amount = 100): userInfo[alice].withdrawalAllowance = 100 totalWithdrawalRequests = 100

2.alice call withdrawalRequest(_amount = 50) again: userInfo[alice].withdrawalAllowance = 50 totalWithdrawalRequests = 150

Impact

maybe lost funds in the first period

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

  function withdrawalRequest(
    uint256 _amount
  ) external nonReentrant onlyWhenVaultIsOn returns (uint256 value) {
    UserInfo storage user = userInfo[msg.sender];
+   require(rebalancingPeriod!=0,"can't request in first period");
    require(user.withdrawalRequestPeriod == 0, "Already a request");

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

    _burn(msg.sender, _amount);

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

Duplicate of #392

sherlock-audit / 2023-01-derby-judging

bin2chen - withdrawalRequest() maybe lost funds in the first period #331