search

sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

ydlee - The collateral redeemed but not collected yet may be borrowed by AMO minter, causing user to fail to collect them. #206

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

ydlee

medium

The collateral redeemed but not collected yet may be borrowed by AMO minter, causing user to fail to collect them.

Summary

The collaterals redeemed but not collected yet by user should stay in the pool, and wait for user to collect them. But AMO minter still can borrow that collaterals, causing user to fail to collect them.

Vulnerability Detail

AMO minter can borrow the collateral from the ubiquity pool, as long as the borrowed amount does not exceed the pool's balance of that collateral.The balance of the coolateral in the pool includes the amount that has been redeemed by users. 

574:    function amoMinterBorrow(uint256 collateralAmount) internal onlyAmoMinter {
575:        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();
576:
577:        // checks the collateral index of the minter as an additional safety check
578:        uint256 minterCollateralIndex = IDollarAmoMinter(msg.sender)
579:            .collateralIndex();
580:
581:        // checks to see if borrowing is paused
582:        require(
583:            poolStorage.isBorrowPaused[minterCollateralIndex] == false,
584:            "Borrowing is paused"
585:        );
586:
587:        // ensure collateral is enabled
588:        require(
589:            poolStorage.isCollateralEnabled[
590:                poolStorage.collateralAddresses[minterCollateralIndex]
591:            ],
592:            "Collateral disabled"
593:        );
594:
595:        // transfer
596:->      IERC20(poolStorage.collateralAddresses[minterCollateralIndex])
597:            .safeTransfer(msg.sender, collateralAmount);
598:    }

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

If balance - unclaimedAmount < amoMinterBorrowAmount <= balance, the borrow transaction will succeed. But subsequent collection transactions will fail, as the pool's balance of the collateral will be insufficient for the collect amount.

Function: contracts/src/dollar/libraries/LibUbiquityPool.sol#collectRedemption()

494:        bool sendCollateral = false;
495:
496:        if (
497:            poolStorage.redeemCollateralBalances[msg.sender][collateralIndex] >
498:            0
499:        ) {
500:->          collateralAmount = poolStorage.redeemCollateralBalances[msg.sender][
501:                collateralIndex
502:            ];
503:            poolStorage.redeemCollateralBalances[msg.sender][
504:                collateralIndex
505:            ] = 0;
506:            poolStorage.unclaimedPoolCollateral[collateralIndex] = poolStorage
507:                .unclaimedPoolCollateral[collateralIndex]
508:                .sub(collateralAmount);
509:            sendCollateral = true;
500:        }
511:
512:        // send out the tokens
513:        if (sendCollateral) {
514:->          IERC20(poolStorage.collateralAddresses[collateralIndex])
515:                .safeTransfer(msg.sender, collateralAmount);
516:        }

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L494-L516

Impact

The collaterals redeemed but not collected yet may be borrowed by AMO minter, causing user to fail to collect them.

Code Snippet

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

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L494-L516

Tool used

Manual Review

Recommendation

While AMO borrowing collaterals, check that the borrow amount is not greater than the balance minus the unclaimed amount.

Duplicate of #1