sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

Varun_05 - Should not be allowed to mint dollar when collateral needed to mint dollar is equal to zero. #207

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

Varun_05

high

Should not be allowed to mint dollar when collateral needed to mint dollar is equal to zero.

Summary

A user can mint dollar tokens for free and if the price of the collateral index decreases then he can even get collateral by redeeming the dollar tokens.This vulnerability is different from the one i submitted earlier because , even if the user is enforced to use the collateral indexes same in mint and redeem dollar function,he can get dollar tokens for free and use that tokens to get the collateral tokens for free.

Vulnerability Detail

Following is the mintDollar function in UbiquityPoolFacet.sol

function mintDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 dollarOutMin,
        uint256 maxCollateralIn
    ) external returns (uint256 totalDollarMint, uint256 collateralNeeded) {
        return
            LibUbiquityPool.mintDollar(
                collateralIndex,
                dollarAmount,
                dollarOutMin,
                maxCollateralIn
            );
    }

Which calls the mintDollar in LibUbiquityPool.sol which is as follows

function mintDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 dollarOutMin,
        uint256 maxCollateralIn
    )
        internal
        collateralEnabled(collateralIndex)
        returns (uint256 totalDollarMint, uint256 collateralNeeded)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        require(
            poolStorage.isMintPaused[collateralIndex] == false,
            "Minting is paused"
        );

        // update Dollar price from Curve's Dollar Metapool
        LibTWAPOracle.update();
        // prevent unnecessary mints
        require(
            getDollarPriceUsd() >= poolStorage.mintPriceThreshold,
            "Dollar price too low"
        );

        // update collateral price
        updateChainLinkCollateralPrice(collateralIndex);

        // get amount of collateral for minting Dollars
        collateralNeeded = getDollarInCollateral(collateralIndex, dollarAmount);

        // subtract the minting fee
        totalDollarMint = dollarAmount
            .mul(
                UBIQUITY_POOL_PRICE_PRECISION.sub(
                    poolStorage.mintingFee[collateralIndex]
                )
            )
            .div(UBIQUITY_POOL_PRICE_PRECISION);

        // check slippages
        require((totalDollarMint >= dollarOutMin), "Dollar slippage");
        require((collateralNeeded <= maxCollateralIn), "Collateral slippage");

        // check the pool ceiling
        require(
            freeCollateralBalance(collateralIndex).add(collateralNeeded) <=
                poolStorage.poolCeilings[collateralIndex],
            "Pool ceiling"
        );

        // take collateral first
        IERC20(poolStorage.collateralAddresses[collateralIndex])
            .safeTransferFrom(msg.sender, address(this), collateralNeeded);

        // mint Dollars
        IERC20Ubiquity ubiquityDollarToken = IERC20Ubiquity(
            LibAppStorage.appStorage().dollarTokenAddress
        );
        ubiquityDollarToken.mint(msg.sender, totalDollarMint);
    }

main issue is in the function which is used to collateral needed in order to mint dollar

 // get amount of collateral for minting Dollars
        collateralNeeded = getDollarInCollateral(collateralIndex, dollarAmount);

getDollarInCollateralIndex function is as follows

 function getDollarInCollateral(
        uint256 collateralIndex,
        uint256 dollarAmount
    ) internal view returns (uint256) {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
        return
            dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(10 ** poolStorage.missingDecimals[collateralIndex])
                .div(poolStorage.collateralPrices[collateralIndex]);
    }

So lets assume missing decimals = 0 so the collateral needed will be calculated as follows

dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(poolStorage.collateralPrices[collateralIndex]);

Lets assume collateral price = 20 dollars i.e poolStorage.collateralPrices[collateralIndex] = 20*10**6 = 2e7 and if dollar amount = 19 then collateral needed will be equal to 19.mul(e6).div(20e6) = 0 So the user can mint dollar tokens for free. Not only this now user has 19 dollar tokens - minting fees so lets assume user got 18 dollar tokens so now lets assume that the user can only redeem dollar for the collateralIndex which while minting dollar tokens.Now if the value of the collateral decreased so now if the redeem function is called which is as follows

    /// @inheritdoc IUbiquityPool
    function redeemDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 collateralOutMin
    ) external returns (uint256 collateralOut) {
        return
            LibUbiquityPool.redeemDollar(
                collateralIndex,
                dollarAmount,
                collateralOutMin
            );
    }

Now lets assume that the price of collateral has decreased to 10 dollars from 20 dollars so now if the redeem is called then the amount of collateral the users would receive is calculated as follows

dollarAmount
                .mul(UBIQUITY_POOL_PRICE_PRECISION)
                .div(poolStorage.collateralPrices[collateralIndex]);

Lets assume after applying redemption fee the amount of dollar = 16 So the collateral he would receive is equal to 16.mul(e6).div(10e6) = 1 collateral token So it can be seen collateral can be recived without initially paying for minting the dollar token.

Impact

This can cause free minting of dollar tokens as well as loss of collateral from the contract which other users have transferred.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/UbiquityPoolFacet.sol#L92C1-L106C1 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/facets/UbiquityPoolFacet.sol#L77 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L284

Tool used

Manual Review

Recommendation

Add the following

function mintDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 dollarOutMin,
        uint256 maxCollateralIn
    )
        internal
        collateralEnabled(collateralIndex)
        returns (uint256 totalDollarMint, uint256 collateralNeeded)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        require(
            poolStorage.isMintPaused[collateralIndex] == false,
            "Minting is paused"
        );

        // update Dollar price from Curve's Dollar Metapool
        LibTWAPOracle.update();
        // prevent unnecessary mints
        require(
            getDollarPriceUsd() >= poolStorage.mintPriceThreshold,
            "Dollar price too low"
        );

        // update collateral price
        updateChainLinkCollateralPrice(collateralIndex);

        // get amount of collateral for minting Dollars
        collateralNeeded = getDollarInCollateral(collateralIndex, dollarAmount);
@=>    require(collateralNeeded >0, "can't mint");
        // subtract the minting fee
        totalDollarMint = dollarAmount
            .mul(
                UBIQUITY_POOL_PRICE_PRECISION.sub(
                    poolStorage.mintingFee[collateralIndex]
                )
            )
            .div(UBIQUITY_POOL_PRICE_PRECISION);

        // check slippages
        require((totalDollarMint >= dollarOutMin), "Dollar slippage");
        require((collateralNeeded <= maxCollateralIn), "Collateral slippage");

        // check the pool ceiling
        require(
            freeCollateralBalance(collateralIndex).add(collateralNeeded) <=
                poolStorage.poolCeilings[collateralIndex],
            "Pool ceiling"
        );

        // take collateral first
        IERC20(poolStorage.collateralAddresses[collateralIndex])
            .safeTransferFrom(msg.sender, address(this), collateralNeeded);

        // mint Dollars
        IERC20Ubiquity ubiquityDollarToken = IERC20Ubiquity(
            LibAppStorage.appStorage().dollarTokenAddress
        );
        ubiquityDollarToken.mint(msg.sender, totalDollarMint);
    }

Duplicate of #7

sherlock-admin2 commented 10 months ago

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

auditsea commented:

$1e-6 can be freely minted, can't even compensate gas

sherlock-admin2 commented 10 months ago

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

auditsea commented:

$1e-6 can be freely minted, can't even compensate gas