coffiasd - AmoMinter borrow collateral assets can lead to protocol DOS

[sherlock-admin2]

coffiasd

medium

AmoMinter borrow collateral assets can lead to protocol DOS

Summary

AmoMinter can borrow collateral token from pool , however not check unclaimed pool collateral which can lead to 3 external functions DOS:

• mintDollar
• collateralUsdBalance
• collectRedemption

Vulnerability Detail

admin can add amoMinter by invoking addAmoMinter
https://github.com/sherlock-audit/2023-12-ubiquity/tree/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L608-L621

    function addAmoMinter(address amoMinterAddress) internal {
        require(amoMinterAddress != address(0), "Zero address detected");

        // make sure the AMO Minter has collateralDollarBalance()
        uint256 collatValE18 = IDollarAmoMinter(amoMinterAddress)
            .collateralDollarBalance();
        require(collatValE18 >= 0, "Invalid AMO");

        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        poolStorage.isAmoMinterEnabled[amoMinterAddress] = true;

        emit AmoMinterAdded(amoMinterAddress);
    }

after amoMinter is added,amoMinter protocol can borrow collateral asset from pool:
https://github.com/sherlock-audit/2023-12-ubiquity/tree/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L574-L598

    function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        // checks the collateral index of the minter as an additional safety check
        uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
            .collateralIndex();

        // checks to see if borrowing is paused
        require(
            poolStorage.isBorrowPaused[minterCollateralIndex] == false,
            "Borrowing is paused"
        );

        // ensure collateral is enabled
        require(
            poolStorage.isCollateralEnabled[
                poolStorage.collateralAddresses[minterCollateralIndex]
            ],
            "Collateral disabled"
        );

        // transfer
        IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
            .safeTransfer(msg.sender, collateralAmount);
    }

however the amount of collateral token amoMinter can borrow not checked , if the amount of collateral token amoMinter borrow bigger than balanceOf(pool) - unclaimedPoolCollateral can lead to DOS due to underflow panic.

Here is my test written using foundry , let's assume:

• alice mint 3e18 dollar
• alice redeem collateral token
• amoMinter borrow 3e18 collateral token from pool

then 3 external functions could be DOS

    function testTrackCollateralBalanceChange() public {
        address alice = makeAddr("alice");

        vm.prank(admin);
        ubiquityPoolFacet.setPriceThresholds(
            1000000, // mint threshold
            1000000 // redeem threshold
        );
        vm.stopPrank();

        //==========================alice mint some collateral.=======================//
        collateralToken.mint(alice,10e18);

        vm.startPrank(alice);
        collateralToken.approve(address(ubiquityPoolFacet),type(uint256).max);

        //==================user deposit collateral and mint Dollar====================//
        (uint256 userMint,) = ubiquityPoolFacet.mintDollar(0,3e18,0,3e18);

        //==============================alice redeem.==================================//
        ubiquityPoolFacet.redeemDollar(0,userMint,0);

        console2.log("pool collateral balance:",collateralToken.balanceOf(address(ubiquityPoolFacet)));
        console2.log("alice collateral balance:",collateralToken.balanceOf(address(alice)));
        console2.log("freeCollateralBalance:",ubiquityPoolFacet.freeCollateralBalance(0));

        //========================amo borrow assets from pool.==========================//
        vm.stopPrank();
        vm.prank(address(dollarAmoMinter));
        ubiquityPoolFacet.amoMinterBorrow(3e18);

        assertEq(collateralToken.balanceOf(address(dollarAmoMinter)),3e18);
        console2.log("amo minter balance:",collateralToken.balanceOf(address(dollarAmoMinter)));

        //=========================pool mintDollar DOS.==================================//
        vm.prank(alice);
        vm.expectRevert(stdError.arithmeticError);
        ubiquityPoolFacet.mintDollar(0,3e18,0,3e18);

        //=========================check collateralUsdBalance DOS.=======================//
        vm.expectRevert(stdError.arithmeticError);
        ubiquityPoolFacet.collateralUsdBalance();

        //add block.
        vm.roll(block.number + 10);
        vm.prank(alice);
        vm.expectRevert("ERC20: transfer amount exceeds balance");
        ubiquityPoolFacet.collectRedemption(0);
    }

output:

@MacBook-Air contracts:: forge test --match-test testTrackCollateralBalanceChange -vvv
[⠆] Compiling...
[⠒] Compiling 1 files with 0.8.19
[⠢] Solc 0.8.19 finished in 17.44s
Compiler run successful!

Running 1 test for test/diamond/facets/UbiquityPoolFacet.t.sol:UbiquityPoolFacetTest
[PASS] testTrackCollateralBalanceChange() (gas: 417597)
Logs:
  pool collateral balance: 3000000000000000000
  alice collateral balance: 7000000000000000000
  freeCollateralBalance: 89400000000000000
  amo minter balance: 3000000000000000000

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 665.79ms

Impact

3 external functions DOS:

• mintDollar
• collateralUsdBalance
• collectRedemption

admin have to deposit collateral token to pool without mint any dollar

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity-coffiasd/tree/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L574-L598

    function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        // checks the collateral index of the minter as an additional safety check
        uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
            .collateralIndex();

        // checks to see if borrowing is paused
        require(
            poolStorage.isBorrowPaused[minterCollateralIndex] == false,
            "Borrowing is paused"
        );

        // ensure collateral is enabled
        require(
            poolStorage.isCollateralEnabled[
                poolStorage.collateralAddresses[minterCollateralIndex]
            ],
            "Collateral disabled"
        );

        // transfer
        IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
            .safeTransfer(msg.sender, collateralAmount);
    }

Tool used

Foundry
Manual Review

Recommendation

the amount of collateral token amoMinter borrow should be checked not bigger than balanceOf(pool) -unclaimedPoolCollateral

Duplicate of #1