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

0 stars 0 forks source link

LiquidationDistributor::distribute will transfer Loan contract(MultiSourceLoan)’s protocol fees to lenders, causing MultiSourceLoan lose protocolFee earnings. #116

Closed c4-bot-1 closed 5 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/pixeldaogg/florida-contracts/blob/7212bfbe9f78ca6eabb5eec86e24d754feb47f15/src/lib/LiquidationDistributor.sol#L112 https://github.com/pixeldaogg/florida-contracts/blob/7212bfbe9f78ca6eabb5eec86e24d754feb47f15/src/lib/LiquidationDistributor.sol#L63

Vulnerability details

Impacts

LiquidationDistributor::distribute will transfer Loan contract(MultiSourceLoan)’s protocol fees to lenders, causing MultiSourceLoan to lose protocolFee earnings.

Proof of concept

In MultiSourceLoan.sol, during Loan initiation (emitLoan) and Loan closing (repayLoan), a protocol fee is charged. For example, (1) In emitLoan flow, in _handleProtocolFeeForFee(), an additional amount is transferred from the lender to protocolFee.recipient. (2) In repayLoan flow, in _processRepayments(), protocolFee is deducted from the borrower’s repayment amount and transferred to the procotolFee.recipient.

//src/lib/loans/MultiSourceLoan.sol
    function _processRepayments(Loan calldata loan) private returns (uint256, uint256) {
...
            uint256 repayment = tranche.principalAmount + tranche.accruedInterest + newInterest - thisProtocolFee;

The problem is in the liquidation settlement flow, the protocolFee is not not deducted from auction proceeds and not transferred to MultiSourceLoan’s designated fee recipient. MultiSrouceLoan loses protocolFee.

Currently, LiquidationDistributor::distribute will transfer the entire auction proceeds to lenders. We see in distribute(), owedPerTranche[i] is calculated with total principalAmount, accured and pending interests, no protocolFee calculation and deduction.

//src/lib/LiquidationDistributor.sol
    function distribute(uint256 _proceeds, IMultiSourceLoan.Loan calldata _loan) external {
...
        for (uint256 i = 0; i < totalTranches;) {
...
|>          owedPerTranche[i] += thisTranche.principalAmount + thisTranche.accruedInterest + pendingInterest;
            unchecked {
                ++i;
            }
        }
...

(https://github.com/pixeldaogg/florida-contracts/blob/7212bfbe9f78ca6eabb5eec86e24d754feb47f15/src/lib/LiquidationDistributor.sol#L63)

In _handleTrancheExcess(), total princiapl + total interests + excess are sent to each lender.

    function _handleTrancheExcess(
        address _tokenAddress,
        IMultiSourceLoan.Tranche calldata _tranche,
        address _liquidator,
        uint256 _proceeds,
        uint256 _totalOwed,
        uint256 _loanEndTime,
        uint256 _protocolFee
    ) private {
...
        /// Total = principal + accruedInterest +  pendingInterest + pro-rata remainder
        uint256 owed = _tranche.principalAmount + _tranche.accruedInterest
            + _tranche.principalAmount.getInterest(_tranche.aprBps, _loanEndTime - _tranche.startTime);
        uint256 total = owed + excess.mulDivDown(owed, _totalOwed);
...
        ERC20(_tokenAddress).safeTransferFrom(_liquidator, _tranche.lender, total);

(https://github.com/pixeldaogg/florida-contracts/blob/7212bfbe9f78ca6eabb5eec86e24d754feb47f15/src/lib/LiquidationDistributor.sol#L112)

In _handleTrancheInsufficient(), the owedPerTranche[i] calculated from distribute() will be passed and transferred to each lender without protocoFee deduction.

    function _handleTrancheInsufficient(
        address _tokenAddress,
        IMultiSourceLoan.Tranche calldata _tranche,
        address _liquidator,
        uint256 _proceedsLeft,
 |>     uint256 _trancheOwed, // owedPerTranche[i]
        uint256 _protocolFee
    ) private returns (uint256) {
...
            ERC20(_tokenAddress).safeTransferFrom(_liquidator, _tranche.lender, _trancheOwed);
...

After distribute() call, AuctionLoanLiquidator::settleAuction will call MultiSourceLoan::loanLiquidated, which will directly delete the loan info.

NulstiSourceLoan will not receive protocolFee.

Tools

Manual

Recommendations

Consider sending the protocolFee portion to multisourceLoan in LiquidationDistributor::distribute. In MultiSourceLoan::loanLiquidated, transfer the received protocolFee to the designated fee recipient before deleting the loan.

Assessed type

Other

0xend commented 5 months ago

Loan fees are only charged in successfully repaid loans. In the future we might add a fee at the auction level (would also avoid an extra ERC20 transfer), but apprecaite you bringing it up.

alex-ppg commented 5 months ago

The Warden specifies that a protocol fee is not charged during loan liquidations. In general, this is acceptable behavior for lending protocols and would make liquidations more lucrative for auction participants. A fixed fee (or something similar) could in theory be implemented, but it is in the best interest of the protocol to maximize the gains of a liquidation rendering this submission to be a recommendation rather than an actual vulnerability.

c4-judge commented 5 months ago

alex-ppg marked the issue as unsatisfactory: Overinflated severity

c4-judge commented 5 months ago

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