Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

TimeLock can prevent ERC-4626 compliance due to setting a lowered deposit cap #307

Closed rokinot closed 1 year ago

rokinot commented 1 year ago

Affected smart contract

BaseVault.sol

Description

BaseVault.sol has a function which allows the timelock to input a deposit cap, setDepositCap(uint256). The function disallows setting the cap as zero or below the minimum vault amount, however, it does not prevent the timelock from depositing an amount lower than currently stored in the vault.

This will prevent maxDeposit() and maxMint() view functions from working as intended, breaking composability and possibly crippling smart contracts interacting with the protocol. Also, as per the ERC-4626 standard, both of these functions must not revert. Additionally, mint() and deposit() will revert due to calling maxMint() (the functions would already revert in a scenario where the total assets stored reached the deposit cap, but in this case it reverts with an arithmetic overflow rather than their custom errors).

Attack scenario

User deposits 1 WETH in a vault

The timelock decreases the maximum deposit cap to 1e18 - 1, equivalent to 0.999... WETH

Future calls to these view functions will revert.

Recommendation

Modify maxDeposit() to include a conditional check to see if the deposit cap is lower than the total assets, and if so return 0.

  function maxDeposit(address) public view virtual override returns (uint256) {
    if(depositCap < totalAssets())
      return 0;
    return depositCap - totalAssets();
  }
0xdcota commented 1 year ago

Deposit cap was removed from vault in pull request #552. This is no longer an issue.