code-423n4 / 2024-05-gondi-mitigation-findings

0 stars 0 forks source link

Some tranche lenders cannot buyout a liquidation even though they have largest principals #118

Closed c4-bot-9 closed 3 months ago

c4-bot-9 commented 3 months ago

Lines of code

https://github.com/pixeldaogg/florida-contracts/blob/7bacbe3f2b4c1bb6c87961e3553118a6e6c2dcee/src/lib/AuctionWithBuyoutLoanLiquidator.sol#L90

Vulnerability details

Impacts

Some tranche lenders cannot buyout a liquidation even though they have largest principals.

Proof of concept

AuctionWithBuyoutLoanLiquidator::settleWithBuyout allows lenders with the largest principal to buy out a liquidation before allowing public bidding.

The problem is there is a vulnerability in settleWithBuyout() that might revert buyout from largest lenders when there are more than one lenders that have equally largest principals.

A loan might have multiple tranches with equally largest principal amounts. In this case settleWithBuyout will not handle the msg.sender(largest lender) checks correctly.

Current settleWithBuyout() will loop through each tranche's principalAmount and find the first tranche idx that has the largest amount. Suppose a loan has three tranches and three lenders (each 1e18 principal). This means largestTrancheIdx will be 0.

    function settleWithBuyout(
    ...
            for (uint256 i = 0; i < _loan.tranche.length; ) {
            if (_loan.tranche[i].principalAmount > largestPrincipal) {
                largestPrincipal = _loan.tranche[i].principalAmount;
                largestTrancheIdx = i;
            }
            unchecked {
                ++i;
            }
        }
|>      if (buyer != _loan.tranche[largestTrancheIdx].lender) {
            revert NotMainLenderError();
        }

(https://github.com/pixeldaogg/florida-contracts/blob/7bacbe3f2b4c1bb6c87961e3553118a6e6c2dcee/src/lib/AuctionWithBuyoutLoanLiquidator.sol#L90)

In this example, if a lender from trancheIdx 1 or 2 calls the buyout. Revert condition buyer != _loan.tranche[largestTrancheIdx].lender will be reached, causing tx revert.

Since lender idx 2 and 3 have equally largest principal, they should be allowed buyout as well.

Tools

Manual

Recommendations

Revise the revert condition, to allow the condition when the buyer has equal principal amount of the found largest principal amount. Consider adding buyer index as an input argument.

Assessed type

Other

0xend commented 3 months ago

If a tie, we only let the most senior one buy it. This is intended behavior.

alex-ppg commented 3 months ago

The Warden claims that multiple largest principal providers of a loan should be able to buy it out without public bidding if they have provided the same principal amount, and the Sponsor claims that this is intended behavior as the most senior principal provider should be the one that has this privilege.

I believe that while the Warden's recommendation is "fairest", the Sponsor's approach can be considered the standard business practice for tie-breakers. As such, I cannot consider this a valid medium-risk vulnerability.

c4-judge commented 3 months ago

alex-ppg marked the issue as unsatisfactory: Overinflated severity

c4-judge commented 3 months ago

alex-ppg changed the severity to QA (Quality Assurance)