0xAlix2 - Accumulated vault fees are not accrued before pools removal, forcing them to be lost

[sherlock-admin4]

0xAlix2

High

Accumulated vault fees are not accrued before pools removal, forcing them to be lost

Summary

When users deposit funds into curated vaults, these funds get deposited into different pools, earning interest when borrowers borrow and repay their debts. Vaults have their part in this interest, and at the same time, part of the vault's profit is gone to the vault's creator/fee recipient.

Minting shares to the fee recipient is done in CuratedVaultSetters::_accrueFee, and this function is not publicly accessible, it is only called when depositing/minting/withdrawing/redeeming. When removing a pool from a vault, users are given a "removal time" to withdraw their funds, so if the following 2 conditions are met, vault fees will be lost forever.

• Some borrowers repay their debt to the pool pending removal, i.e. accumulating interest.
• No users withdraw their funds from the vault OR they withdraw before borrowers' repayment

If the above were met, the vault fees would be lost forever, as currently, it is only doable when depositing/minting/withdrawing/redeeming.

Root Cause

Vault fees accumulated from pending removal pools are not being accrued before removing those pools from the withdrawal queue, specifically in updateWithdrawQueue.

Impact

Vault fees accumulated from the removed pools will be lost forever.

PoC

Test/POC

```solidity contract USDCMocked is ERC20, ERC20Permit { constructor() ERC20('USDC', 'USDC') ERC20Permit('USDC') {} function mint(address account, uint256 value) public returns (bool) { _mint(account, value); return true; } function decimals() public pure override returns (uint8) { return 6; } } contract Contest_Vault is Test { PoolFactory public poolFactory; DefaultReserveInterestRateStrategy public irStrategy; PoolConfigurator public configurator; IPool internal pool_1; IPool internal pool_2; ICuratedVault internal vault; ICuratedVaultFactory internal vaultFactory; WETH9Mocked public WETH; USDCMocked public USDC; MockV3Aggregator public WETHOracle; MockV3Aggregator public USDCOracle; address public bob = makeAddr('bob'); address public borrower = makeAddr('borrower'); address public feeRecipient = makeAddr('feeRecipient'); address public owner = makeAddr('owner'); address internal allocator = makeAddr('allocator'); address internal curator = makeAddr('curator'); address internal guardian = makeAddr('guardian'); function _setUpCore() internal { poolFactory = new PoolFactory(address(new Pool())); configurator = new PoolConfigurator(address(poolFactory)); poolFactory.setConfigurator(address(configurator)); WETH = new WETH9Mocked(); USDC = new USDCMocked(); WETHOracle = new MockV3Aggregator(8, 2_600e8); USDCOracle = new MockV3Aggregator(8, 1e8); irStrategy = new DefaultReserveInterestRateStrategy(47 * 1e25, 0, 7 * 1e25, 30 * 1e25); poolFactory.setReserveFactor(500); } function _setupPool1() internal { address[] memory assets = new address[](2); assets[0] = address(WETH); assets[1] = address(USDC); address[] memory rateStrategyAddresses = new address[](2); rateStrategyAddresses[0] = address(irStrategy); rateStrategyAddresses[1] = address(irStrategy); address[] memory sources = new address[](2); sources[0] = address(WETHOracle); sources[1] = address(USDCOracle); DataTypes.InitReserveConfig[] memory configurationLocal = new DataTypes.InitReserveConfig[](2); configurationLocal[0] = DataTypes.InitReserveConfig({ ltv: 7500, liquidationThreshold: 8000, liquidationBonus: 10_500, decimals: 18, frozen: false, borrowable: true, borrowCap: 0, supplyCap: 0 }); configurationLocal[1] = DataTypes.InitReserveConfig({ ltv: 0, liquidationThreshold: 8000, liquidationBonus: 10_500, decimals: 6, frozen: false, borrowable: true, borrowCap: 0, supplyCap: 0 }); address[] memory admins = new address[](1); admins[0] = address(this); DataTypes.InitPoolParams memory p = DataTypes.InitPoolParams({ proxyAdmin: address(this), revokeProxy: false, admins: admins, emergencyAdmins: new address[](0), riskAdmins: new address[](0), hook: address(0), assets: assets, rateStrategyAddresses: rateStrategyAddresses, sources: sources, configurations: configurationLocal }); poolFactory.createPool(p); pool_1 = poolFactory.pools(0); } function _setupPool2() internal { address[] memory assets = new address[](2); assets[0] = address(WETH); assets[1] = address(USDC); address[] memory rateStrategyAddresses = new address[](2); rateStrategyAddresses[0] = address(irStrategy); rateStrategyAddresses[1] = address(irStrategy); address[] memory sources = new address[](2); sources[0] = address(WETHOracle); sources[1] = address(USDCOracle); DataTypes.InitReserveConfig[] memory configurationLocal = new DataTypes.InitReserveConfig[](2); configurationLocal[0] = DataTypes.InitReserveConfig({ ltv: 7500, liquidationThreshold: 8000, liquidationBonus: 10_500, decimals: 18, frozen: false, borrowable: true, borrowCap: 0, supplyCap: 0 }); configurationLocal[1] = DataTypes.InitReserveConfig({ ltv: 8000, liquidationThreshold: 8000, liquidationBonus: 10_500, decimals: 6, frozen: false, borrowable: true, borrowCap: 0, supplyCap: 0 }); address[] memory admins = new address[](1); admins[0] = address(this); DataTypes.InitPoolParams memory p = DataTypes.InitPoolParams({ proxyAdmin: address(this), revokeProxy: false, admins: admins, emergencyAdmins: new address[](0), riskAdmins: new address[](0), hook: address(0), assets: assets, rateStrategyAddresses: rateStrategyAddresses, sources: sources, configurations: configurationLocal }); poolFactory.createPool(p); pool_2 = poolFactory.pools(1); } function _setUpCuratedVault() internal { CuratedVault instance = new CuratedVault(); vaultFactory = ICuratedVaultFactory(new CuratedVaultFactory(address(instance))); address[] memory admins = new address[](1); address[] memory curators = new address[](1); address[] memory guardians = new address[](1); address[] memory allocators = new address[](1); admins[0] = owner; curators[0] = curator; guardians[0] = guardian; allocators[0] = allocator; vault = vaultFactory.createVault( ICuratedVaultFactory.InitVaultParams({ revokeProxy: true, proxyAdmin: owner, admins: admins, curators: curators, guardians: guardians, allocators: allocators, timelock: 1 weeks, asset: address(USDC), name: 'Vault', symbol: 'VLT', salt: keccak256('salty') }) ); vm.startPrank(owner); vault.setFeeRecipient(feeRecipient); vault.setFee(0.05e18); vm.stopPrank(); } function _setCap(IPool pool_, uint256 newCap) internal { vm.prank(curator); vault.submitCap(pool_, newCap); vm.warp(block.timestamp + vault.timelock()); vault.acceptCap(pool_); IPool[] memory newSupplyQueue = new IPool[](vault.supplyQueueLength() + 1); for (uint256 k; k < vault.supplyQueueLength(); k++) newSupplyQueue[k] = vault.supplyQueue(k); newSupplyQueue[vault.supplyQueueLength()] = pool_; vm.prank(allocator); vault.setSupplyQueue(newSupplyQueue); } function _syncOracles() internal { WETHOracle.updateRoundTimestamp(); USDCOracle.updateRoundTimestamp(); } function setUp() public { _setUpCore(); _setupPool1(); _setupPool2(); _setUpCuratedVault(); _setCap(pool_1, 500e6); _setCap(pool_2, 500e6); _syncOracles(); } function testNotAccruingFeesOnRemoval() public { uint256 USDCamount = 1_000e6; uint256 WETHamount = 1e18; USDC.mint(bob, USDCamount); USDC.mint(borrower, USDCamount); WETH.mint(borrower, WETHamount); // Bob deposits 1k USDC into the vault, 500 to pool 1 and 500 to pool 2 vm.startPrank(bob); USDC.approve(address(vault), type(uint256).max); vault.deposit(USDCamount, bob); vm.stopPrank(); // Borrower supplies 1 WETH to pool 1 and borrows 500 USDC vm.startPrank(borrower); WETH.approve(address(pool_1), type(uint256).max); pool_1.supplySimple(address(WETH), borrower, WETHamount, 0); pool_1.borrowSimple(address(USDC), borrower, USDCamount / 2, 0); vm.stopPrank(); // Curator removes sets the cap of pool 1 to 0 vm.startPrank(curator); vault.submitCap(pool_1, 0); vault.submitMarketRemoval(pool_1); vm.stopPrank(); // Some time passes // Some interest is accumulated vm.warp(block.timestamp + vault.timelock()); pool_1.forceUpdateReserves(); // Borrower repays his debt // Some fees should be accrued by the vault vm.startPrank(borrower); USDC.approve(address(pool_1), type(uint256).max); pool_1.repaySimple(address(USDC), pool_1.getDebt(address(USDC), borrower, 0), 0); vm.stopPrank(); // Bob doesn't withdraw his assets from the vault // Allocator removes the pool from the vault uint256[] memory indices = new uint256[](1); indices[0] = 1; vm.prank(allocator); vault.updateWithdrawQueue(indices); // No fees were accrued by the vault assertEq(vault.balanceOf(feeRecipient), 0); } } ```

Mitigation

There are 2 possible solutions:

• Call _accrueFee(); at the top of CuratedVault::updateWithdrawQueue, so the fees are accrued before removing pools.
• Expose a function that calls _accrueFee();, so that accruing could be done by anyone anytime.

Duplicate of #448

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

Honour commented:

same reason as #068. there's a reallocate function to reallocate funds & accrued interest from pools before removing it. Also users depositng into curated vaults have no notion of the underlying pool they're depositng into