sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

Drynooo - User calls to collectRedemption function may fail #106

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

Drynooo

medium

User calls to collectRedemption function may fail

Summary

The user's call to the collectRedemption function may fail due to insufficient funds in the contract.

Vulnerability Detail

Under normal circumstances, users will first call the redeemDollar function, and then call the collectRemption function one day later to withdraw the collateral. However, during this period, AMO may lend out this part of the funds, resulting in insufficient funds in the contract for the user to withdraw, and the user will fail to call the collectRemption function. 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);
    }

LibUbiquityPool.sol#L476-L517

        if (
            poolStorage.redeemCollateralBalances[msg.sender][collateralIndex] >
            0
        ) {
            collateralAmount = poolStorage.redeemCollateralBalances[msg.sender][
                collateralIndex
            ];
            poolStorage.redeemCollateralBalances[msg.sender][
                collateralIndex
            ] = 0;
            poolStorage.unclaimedPoolCollateral[collateralIndex] = poolStorage
                .unclaimedPoolCollateral[collateralIndex]
                .sub(collateralAmount);
            sendCollateral = true;
        }

        // send out the tokens
        if (sendCollateral) {
            IERC20(poolStorage.collateralAddresses[collateralIndex])
                .safeTransfer(msg.sender, collateralAmount);
        }
    }

Impact

Users calling the collectRemption function to withdraw collateral may fail.

Code Snippet

LibUbiquityPool.sol#L574-L598 LibUbiquityPool.sol#L476-L517

Tool used

Manual Review

Recommendation

It is recommended to restrict AMOs from lending funds recorded by the unclaimedPoolCollateral variable.

Duplicate of #1