sherlock-audit / 2023-01-derby-judging

4 stars 1 forks source link

bin2chen - withdraw() users may not be able to withdraw #362

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

bin2chen

medium

withdraw() users may not be able to withdraw

Summary

Forced vault is On may result in inability to withdraw

Vulnerability Detail

Users can use withdraw() to make withdrawals: The code is as follows:

  function withdraw(
    uint256 _amount,
    address _receiver,
    address _owner
  ) external nonReentrant onlyWhenVaultIsOn returns (uint256 value) { //<------vault mus on
    value = (_amount * exchangeRate) / (10 ** decimals());

    require(value > 0, "!value");

    require(getVaultBalance() - reservedFunds >= value, "!funds");

    _burn(msg.sender, _amount);
    transferFunds(_receiver, value);
  }

This method has restrictions on the vault must be On If none of the players vote on this vault in Game.sol it will be set to Off by XChainController

  function settleCurrentAllocation(
    uint256 _vaultNumber,
    uint32 _chainId,
    int256 _deltas
  ) internal returns (uint256 activeVault) {
    if (getCurrentAllocation(_vaultNumber, _chainId) == 0 && _deltas == 0) {
      vaults[_vaultNumber].chainIdOff[_chainId] = true;   //<-----set to Off
      activeVault = 0;
    } else {
      vaults[_vaultNumber].chainIdOff[_chainId] = false;
      activeVault = 1;
    }

But the user deposit, should be allowed to withdraw, whether the player has voted or not

It makes more sense to use onlyWhenIdle

Impact

users may not be able to withdraw

Code Snippet

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

Tool used

Manual Review

Recommendation

  function withdraw(
    uint256 _amount,
    address _receiver,
    address _owner
- ) external nonReentrant onlyWhenVaultIsOn returns (uint256 value) {
+ ) external nonReentrant onlyWhenIdle returns (uint256 value) {
    value = (_amount * exchangeRate) / (10 ** decimals());

    require(value > 0, "!value");

    require(getVaultBalance() - reservedFunds >= value, "!funds");

    _burn(msg.sender, _amount);
    transferFunds(_receiver, value);
  }