0xnirlin - Loss of fee in `ubiquity.sol`

[sherlock-admin2]

0xnirlin

high

Loss of fee in ubiquity.sol

Summary

In ubiquity.sol fee is collected but there is no way to withdraw that fee from the contract and is completely stuck in the contract.

Vulnerability Detail

Fee is collected at two points:

1. In minting, but minting less dollars
2. In redemption by returning less collateral against the dollar.

This means, there while redemption the fee is collected in the form of collateral tokens which remain in Ubiquity.sol() and are not collected as there is no such withdraw function.

Also, there is no way to track how much fee has been collected as it is never stored on chain.

   function redeemDollar(
        uint256 collateralIndex,
        uint256 dollarAmount,
        uint256 collateralOutMin
    )
        internal
        collateralEnabled(collateralIndex)
        returns (uint256 collateralOut)
    {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        require(
            poolStorage.isRedeemPaused[collateralIndex] == false,
            "Redeeming is paused"
        );

        // update Dollar price from Curve's Dollar Metapool
        LibTWAPOracle.update();
        // prevent unnecessary redemptions that could adversely affect the Dollar price
        require(
            getDollarPriceUsd() <= poolStorage.redeemPriceThreshold,
            "Dollar price too high"
        );

        uint256 dollarAfterFee = dollarAmount
            .mul(
                UBIQUITY_POOL_PRICE_PRECISION.sub(
                    poolStorage.redemptionFee[collateralIndex]
                )
            )
            .div(UBIQUITY_POOL_PRICE_PRECISION);

        // update collateral price
        updateChainLinkCollateralPrice(collateralIndex);

        // get collateral output for incoming Dollars
        collateralOut = getDollarInCollateral(collateralIndex, dollarAfterFee);

        // checks
        require(
            collateralOut <=
                (IERC20(poolStorage.collateralAddresses[collateralIndex]))
                    .balanceOf(address(this))
                    .sub(poolStorage.unclaimedPoolCollateral[collateralIndex]),
            "Insufficient pool collateral"
        );
        require(collateralOut >= collateralOutMin, "Collateral slippage");

        // account for the redeem delay
        poolStorage.redeemCollateralBalances[msg.sender][
            collateralIndex
        ] = poolStorage
        .redeemCollateralBalances[msg.sender][collateralIndex].add(
                collateralOut
            );
        poolStorage.unclaimedPoolCollateral[collateralIndex] = poolStorage
            .unclaimedPoolCollateral[collateralIndex]
            .add(collateralOut);

        poolStorage.lastRedeemedBlock[msg.sender] = block.number;

        // burn Dollars
        IERC20Ubiquity ubiquityDollarToken = IERC20Ubiquity(
            LibAppStorage.appStorage().dollarTokenAddress
        );
        ubiquityDollarToken.burnFrom(msg.sender, dollarAmount);
    }

Impact

Fee is stuck.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L399-L517

Tool used

cATS

Recommendation

Track the fee collected and add a withdraw function.

Duplicate of #36

[sherlock-admin2]

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

auditsea commented:

The goal of fee taken for mint/redeem is to hedge the risk of price deviation

[sherlock-admin2]

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

auditsea commented:

The goal of fee taken for mint/redeem is to hedge the risk of price deviation