Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wellbyt3 - Supplying to vault DoS when underlying pools have capacity #193

Closed
sherlock-admin4 opened this issue Sep 10, 2024 · 1 comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 10, 2024

wellbyt3

Medium

Supplying to vault DoS when underlying pools have capacity

Summary

In CurateVaultSetters.sol, _supplyPool() doesn't calculate the capacity available in each pool before attempting to supply to them. This can lead to a situation where _supplyPool() reverts with the CuratedErrorsLib.AllCapsReached() error, preventing suppliers from depositing/minting with the vault while there is actually sufficient capacity.

Note: This is a different root cause and issue from the known issue mentioned on page 34 of the audit report (explained in "Root Cause").

Root Cause

In CuratedVaultSetters.sol:_supplyPool(uint256 assets), toSupply is the minimum value of 1) supplyCap.zeroFloorSub(supplyAssets) or 2) assets, but does not take into account the capacity available in each pool before calling supplySimple() and attempting to supply the pool.

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/vaults/CuratedVaultSetters.sol#L130-L137

When a user tries to supply assets (depositing/minting), the protocol intends for these deposits to fill each pool up to its supplyCap before moving to the next pool in the supplyQueue, but this doesn't happen. Instead, the pool won't receive any of the supply and _supplyPool() will try the next pool in the supplyQueue.

Internal pre-conditions

  1. When a supplier to a vault calls deposit() or mint(), toSupply in _supplyPool(uint256 assets) must exceed the capacity of each pool in the supplyQueue, but be less than the total capacity of all pools combined.

External pre-conditions

n/a

Attack Path

  1. Say Vault has two pools (pool1 and pool2) in its supplyQueue, each having a supplyCap of 10.
  2. The total amount of deposits the vault should be able to accept is 20e18.
  3. A supplier attempts to deposit 11e18.
  4. Instead of 10e18 going to pool1 and 1e18 going to pool2, the deposit reverts throwing the CuratedErrorsLib.AllCapsReached(); error due to _supplyPool() attempting to deposit 11e18 into each pool, exceeding each individual pools capacity.

Impact

Suppliers attempting to deposit() or mint() amounts large enough are unable to use the vault.

PoC

Create a new file in /test/forge/core/vaults/ (e.g. SupplyRevertsTest.t.sol) and paste the following POC into the new file.

Run forge test --mt test_SupplyRevertsWhenSpaceIsStillAvailableInPools -vv

// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {Math} from '@openzeppelin/contracts/utils/math/Math.sol';
import {IERC4626Upgradeable, MathUpgradeable} from '@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol';
import './helpers/IntegrationVaultTest.sol';
import '../pool/CorePoolTests.sol';
import {CuratedErrorsLib, CuratedEventsLib, CuratedVault, PendingUint192} from '../../../../contracts/core/vaults/CuratedVault.sol';
import {CuratedVaultFactory, ICuratedVaultFactory} from '../../../../contracts/core/vaults/CuratedVaultFactory.sol';
import {ICuratedVault} from '../../../../contracts/interfaces/vaults/ICuratedVault.sol';
import {ReserveConfiguration} from '../../../../contracts/core/pool/configuration/ReserveConfiguration.sol';


contract SupplyRevertsWhenSpaceIsStillAvailableInPools is CorePoolTests {
    using ReserveConfiguration for DataTypes.ReserveConfigurationMap;

    bytes32 pos;

    IPool pool1;
    IPool pool2;

    ICuratedVault internal vault;
    ICuratedVaultFactory internal vaultFactory;
    ICuratedVaultFactory.InitVaultParams internal defaultVaultParams;

    MintableERC20 internal loanToken;
    MintableERC20 internal collateralToken;
    MockV3Aggregator internal oracle;

    address internal supplier = makeAddr('supplier');
    address internal borrower = makeAddr('borrower');
    address internal repayer = makeAddr('repayer');
    address internal onBehalf = makeAddr('onBehalf');
    address internal receiver = makeAddr('receiver');
    address internal allocator = makeAddr('allocator');
    address internal curator = makeAddr('curator');
    address internal guardian = makeAddr('guardian');
    address internal feeRecipient = makeAddr('feeRecipient');
    address internal skimRecipient = makeAddr('skimRecipient');

    function setUp() public {
        _setUpVault();
        loanToken.mint(supplier, 1000e18);
    }

    function test_SupplyRevertsWhenSpaceIsStillAvailableInPools() public {
        uint256 supplyCapPool1;
        uint256 supplyCapPool2;
        uint256 totalSupplyCapacity;

        uint256 pool1TotalAssets;
        uint256 pool2TotalAssets;

        // There are two pools: pool1 and pool2. 
        // Each pool has a supplyCap of 10.
        // This is the supply cap set by the pool, not the vault.
        DataTypes.ReserveConfigurationMap memory poolConfig1 = pool1.getConfiguration(address(loanToken));
        DataTypes.ReserveConfigurationMap memory poolConfig2 = pool2.getConfiguration(address(loanToken));

        // Get the supply cap for each pool
        supplyCapPool1 = poolConfig1.getSupplyCap() * (10 ** poolConfig1.getDecimals());
        supplyCapPool2 = poolConfig2.getSupplyCap() * (10 ** poolConfig2.getDecimals());

        // Get total assets of each pool (both are 0)
        pool1TotalAssets = pool1.totalAssets(address(loanToken));
        pool2TotalAssets = pool2.totalAssets(address(loanToken));

        // Calculate total supply capacity
        totalSupplyCapacity = supplyCapPool1 + supplyCapPool2 - pool1TotalAssets - pool2TotalAssets;

        console.log("Pool1 supplyCap: ", supplyCapPool1);
        console.log("Pool2 supplyCap: ", supplyCapPool2);
        console.log("Total Supply Available: ", totalSupplyCapacity);
        console.log("Pool1 totalAssets: ", pool1TotalAssets);
        console.log("Pool2 totalAssets: ", pool2TotalAssets);

        // supplier wants to deposit 11e18 tokens into the vault
        uint256 depositAmount = 11e18;
        vm.startPrank(supplier);
        loanToken.approve(address(vault), type(uint256).max);
        
        require(depositAmount <= totalSupplyCapacity, 'Deposit amount exceeds total supply available');
        
        // totalSupplyCapacity is 20e18, so depositAmount of 11e18 should be valid,
        // but deposit() reverts because depositAmount exceeds the space available in all individual pools
        vm.expectRevert();
        uint256 shares = vault.deposit(depositAmount, supplier);

        assertEq(shares,0);
    }

    function _setUpVault() public {
         _setUpSupplyCappedPool();

        loanToken = tokenA;
        collateralToken = tokenB;
        oracle = oracleA;

        vm.label(address(loanToken), 'loanToken');
        vm.label(address(collateralToken), 'collateralToken');
        vm.label(address(oracle), 'oracle');
        vm.label(address(irStrategy), 'irStrategy');
        oracle.updateAnswer(1e8);

        CuratedVault instance = new CuratedVault();
        vaultFactory = ICuratedVaultFactory(new CuratedVaultFactory(address(instance)));

        // setup the default vault params
        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;
        guardians[0] = allocator;

        defaultVaultParams = ICuratedVaultFactory.InitVaultParams({
        revokeProxy: true,
        proxyAdmin: owner,
        admins: admins,
        curators: curators,
        guardians: guardians,
        allocators: allocators,
        timelock: 1 weeks,
        asset: address(loanToken),
        name: 'Vault',
        symbol: 'VLT',
        salt: keccak256('salty')
        });

        vault = vaultFactory.createVault(defaultVaultParams);

        vm.startPrank(owner);
        vault.grantCuratorRole(curator);
        vault.grantAllocatorRole(allocator);
        vault.setFeeRecipient(feeRecipient);
        vault.setSkimRecipient(skimRecipient);
        vm.stopPrank();

        // set the vault's pool cap
        _setCap(pool1, type(uint184).max); 
        _setCap(pool2, type(uint184).max); 

        IPool[] memory supplyQueue = new IPool[](2);
        supplyQueue[0] = pool1;
        supplyQueue[1] = pool2;

        vm.prank(allocator);
        vault.setSupplyQueue(supplyQueue);

        oracle.updateRoundTimestamp();
    }

    function _setUpSupplyCappedPool() internal {
        _setUpCorePool();
        poolFactory.createPool(_supplyCappedPoolInitParams());
        IPool supplyCappedPoolAddr = poolFactory.pools(0);
        pool1 = IPool(address(supplyCappedPoolAddr));
        
        poolFactory.createPool(_supplyCappedPoolInitParams());
        IPool supplyCappedPoolAddr2 = poolFactory.pools(1);
        pool2 = IPool(address(supplyCappedPoolAddr2));

        pos = keccak256(abi.encodePacked(address(owner), 'index', uint256(0)));
    }

    function _supplyCappedPoolInitParams() internal view returns (DataTypes.InitPoolParams memory p) {
        address[] memory assets = new address[](4);
        assets[0] = address(tokenA);
        assets[1] = address(tokenB);
        assets[2] = address(tokenC);
        assets[3] = address(wethToken);

        address[] memory rateStrategyAddresses = new address[](4);
        rateStrategyAddresses[0] = address(irStrategy);
        rateStrategyAddresses[1] = address(irStrategy);
        rateStrategyAddresses[2] = address(irStrategy);
        rateStrategyAddresses[3] = address(irStrategy);

        address[] memory sources = new address[](4);
        sources[0] = address(oracleA);
        sources[1] = address(oracleB);
        sources[2] = address(oracleC);
        sources[3] = address(oracleD);

        DataTypes.InitReserveConfig memory config = _SupplyCappedPoolConfig();

        DataTypes.InitReserveConfig[] memory configurationLocal = new DataTypes.InitReserveConfig[](4);
        configurationLocal[0] = config;
        configurationLocal[1] = config;
        configurationLocal[2] = config;
        configurationLocal[3] = config;

        address[] memory admins = new address[](1);
        admins[0] = address(this);

        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 // @note where supplyCap is set
        });
    }

    function _SupplyCappedPoolConfig() internal pure returns (DataTypes.InitReserveConfig memory c) {
        c = DataTypes.InitReserveConfig({
        ltv: 7500,
        liquidationThreshold: 8000,
        liquidationBonus: 10_500,
        decimals: 18,
        frozen: false,
        borrowable: true,
        borrowCap: 0,
        supplyCap: 10 // Set the pool supplyCap
        });
    }


    function _setCap(IPool _pool, uint256 newCap) internal {
        uint256 cap = vault.config(_pool).cap;
        bool isEnabled = vault.config(_pool).enabled;
        if (newCap == cap) return;

        PendingUint192 memory pendingCap = vault.pendingCap(_pool);
        if (pendingCap.validAt == 0 || newCap != pendingCap.value) {
        vm.prank(curator);
        vault.submitCap(_pool, newCap);
        }

        if (newCap < cap) return;

        vm.warp(block.timestamp + vault.timelock());

        vault.acceptCap(_pool);

        assertEq(vault.config(_pool).cap, newCap, '_setCap');

        if (newCap > 0) {
        if (!isEnabled) {
            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);
        }
        }
    }
}

Mitigation

Modify the toSupply variable in _supplyPool() to take into account the capacity the pool has available.

https://github.com/sherlock-audit/2024-06-new-scope/blob/main/zerolend-one/contracts/core/vaults/CuratedVaultSetters.sol#L130

Duplicate of #433

@github-actions github-actions bot added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label Sep 20, 2024
@sherlock-admin4
Copy link
Contributor Author

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

Honour commented:

Invalid: misleading POC. sets pools (vault) caps to uint256.max while actual pool caps are 10e18. setCap is an admin operation and It can be expected that the curators will set (there's no reason to assume otherwise) a similar pool(vault) cap as the underlying pool

@sherlock-admin3 sherlock-admin3 changed the title Innocent Yellow Cheetah - Supplying to vault DoS when underlying pools have capacity wellbyt3 - Supplying to vault DoS when underlying pools have capacity Oct 3, 2024
@sherlock-admin3 sherlock-admin3 added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Oct 3, 2024
@WangSecurity WangSecurity added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. labels Oct 26, 2024
@sherlock-admin2 sherlock-admin2 added Reward A payout will be made for this issue and removed Non-Reward This issue will not receive a payout labels Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A Medium severity issue. Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

4 participants