code-423n4 / 2024-04-gondi-findings

0 stars 0 forks source link

`triggerFee` is stolen from other auctions during `settleWithBuyout()` #50

Open c4-bot-5 opened 7 months ago

c4-bot-5 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/AuctionWithBuyoutLoanLiquidator.sol#L97

Vulnerability details

Impact

The function settleWithBuyout() is used to settle an auction with a buyout from the main lender. This lender needs to repay all other lenders and will receive the NFT collateral. Near the end of the function, the triggerFee is also paid to the auction originator. However, the funds used to pay this fee are taken directly from the contract balance, even though the main lender doesn't transfer these funds into the contract.

function settleWithBuyout(
    ...
) external nonReentrant {
    ...
    // @note Repay other lenders
    ERC20 asset = ERC20(_auction.asset); 
    uint256 totalOwed;
    for (uint256 i; i < _loan.tranche.length;) {
        ...
    }
    IMultiSourceLoan(_auction.loanAddress).loanLiquidated(_auction.loanId, _loan);

    // @audit There is no fund in this contract to pay triggerFee
    asset.safeTransfer(_auction.originator, totalOwed.mulDivDown(_auction.triggerFee, _BPS)); 
    ...
}

As a result, if the auction contract balance is insufficient to cover the fee, the function will simply revert and prevent the main lender from buying out. In other cases where multiple auctions are running in parallel, the triggerFee will be deducted from the other auctions. This could lead to the last auctions being unable to settle due to insufficient balance.

Proof of Concept

The function settleWithBuyout() is called before any placeBid() so the funds is only from main lender. In the settleWithBuyout(), there are 2 transfers asset. One is to pay other lenders and one is to pay the triggerFee. As you can see in the code snippet, there is no triggerFee transfer from sender to originator.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using safeTransferFrom() to pay the triggerFee from the sender's address, rather than using safeTransfer() to pay the triggerFee from the contract balance.

Assessed type

Other

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

0xA5DF commented 7 months ago

TODO: severity

c4-judge commented 7 months ago

0xA5DF marked the issue as selected for report

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory

0xA5DF commented 7 months ago

Sustaining high severity because this is going to cause a loss of principal to other auctions

0xend commented 7 months ago

https://github.com/pixeldaogg/florida-contracts/pull/370