0xAlix2 - AmoMinter can borrow collateral more than what's free/available

[sherlock-admin]

0xAlix2

medium

AmoMinter can borrow collateral more than what's free/available

Summary

AmoMinters can borrow collateral from the protocol to earn yield in external protocols like Compound, and Curve, ... This can be done using the amoMinterBorrow function, which sends the "amount" of collateral to the targeted AmoMinter. However, this function doesn't check the validity of the borrowed amount, i.e. it doesn't check if the protocol has enough "free" amount before going ahead and sending the collateral.

Vulnerability Detail

AmoMinter can borrow a collateral amount even if it is included in an unclaimed collateral storage. So if a user calls redeemDollar on certain collateral, and an AmoMinter borrows the full amount of collateral, that user won't be able to call collectRedemption, forcing him to lose his dollar tokens for nothing in return. As the transferred "borrowed" amount won't consider the unclaimed collateral unclaimedPoolCollateral[collateralIndex].

function test_bug_noFreeCollateralCheck() public {
    uint256 _100ETH = 100e18;

    vm.prank(admin);
    ubiquityPoolFacet.setPriceThresholds(1000000, 1000000);

    assertEq(collateralToken.balanceOf(address(ubiquityPoolFacet)), 0);

    vm.prank(user);
    ubiquityPoolFacet.mintDollar(0, _100ETH, 99e18, _100ETH);

    assertEq(
        collateralToken.balanceOf(address(ubiquityPoolFacet)),
        _100ETH
    );

    vm.prank(user);
    ubiquityPoolFacet.redeemDollar(0, 99e18, 90e18);

    assertEq(
        collateralToken.balanceOf(address(ubiquityPoolFacet)),
        _100ETH
    );
    assertEq(ubiquityPoolFacet.freeCollateralBalance(0), 2.98e18);

    vm.prank(address(dollarAmoMinter));
    ubiquityPoolFacet.amoMinterBorrow(_100ETH);

    vm.roll(3);

    vm.prank(user);
    vm.expectRevert("ERC20: transfer amount exceeds balance");
    ubiquityPoolFacet.collectRedemption(0);
}

Impact

Loss of funds (dollar tokens).

Code Snippet

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

Tool used

Manual Review + vscode

Recommendation

Add the following require statement in the amoMinterBorrow function in the LibUbiquityPool library.

require(
    collateralAmount <= freeCollateralBalance(minterCollateralIndex),
    "Not enough free collateral"
);

[sherlock-admin2]

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

0xLogos commented:

amo minter is trusted, imho correctness of amo behavior is vital to protocol solvency and there's no way to safeguard against its bad behavior and keep its functionality because allowing him to withdraw all free collateral barely only protects pending withdrawls

[sherlock-admin]

The protocol team fixed this issue in PR/commit ubiquity/ubiquity-dollar#882.

[molecula451]

/start

[ubiquibot]

Warning!	This task was created over 36 days ago. Please confirm that this issue specification is accurate before starting.
Deadline	Sat, Feb 17, 2:55 PM UTC
Registered Wallet	0x4D0704f400D57Ba93eEa88765C3FcDBD826dCFc4

Tips:

• Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
• Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
• Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

[ubiquibot]

+ Evaluating results. Please wait...

[ubiquibot]

# No linked pull requests to close

[molecula451]

cross-side ref PR by rndqnuu #1 (comment)

[ubiquibot]

! No price label has been set. Skipping permit generation.

[ubiquibot]

+ Evaluating results. Please wait...

[ubiquibot]

[ 3 WXDAI ]

@molecula451

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	2	3

Conversation Incentives

Comment	Formatting	Relevance	Reward
PR Fix Confirmation: https://github.com/ubiquity/ubiquity-dollar...	1.1	0.39	1.1
cross-side ref PR by rndqnuu https://github.com/sherlock-audit/2...	1.9	0.85	1.9

[ 2.5 WXDAI ]

@gitcoindev

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	1	2.5

Conversation Incentives

Comment	Formatting	Relevance	Reward
@rndquu I agree with your comment. Since we do not use AMO minte...	2.5	0.51	2.5

[ 104.4 WXDAI ]

@rndquu

Contributions Overview

View	Contribution	Count	Reward
Issue	Task	1.00	100
Issue	Comment	1	0
Issue	Comment	1	4.4

Conversation Incentives

Comment	Formatting	Relevance	Reward
@gitcoindev @molecula451 AMO minters are trusted so it's bas...	-	0.5	-
@gitcoindev @molecula451 AMO minters are trusted so it's bas...	4.4	0.5	4.4

[sherlock-admin]

The Lead Senior Watson signed off on the fix.