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

0 stars 0 forks source link

distribute() Use the wrong end time to break maxSeniorRepayment's expectations #16

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/LiquidationDistributor.sol#L39

Vulnerability details

Vulnerability details

When the bid amount is not enough, the lender will be repaid in order of tranche[]. In order to minimize the risk, the user can specify maxSeniorRepayment to avoid the risk to some extent, and put himself in a position of higher repayment priority. At the same time emitLoan() checks maxSeniorRepayment for the emitLoan()

emitLoan() -> _processOffersFromExecutionData() -> _checkOffer()

    function _processOffersFromExecutionData(
        address _borrower,
        address _principalReceiver,
        address _principalAddress,
        address _nftCollateralAddress,
        uint256 _tokenId,
        uint256 _duration,
        OfferExecution[] calldata _offerExecution
    ) private returns (uint256, uint256[] memory, Loan memory, uint256) {
...
            uint256 amount = thisOfferExecution.amount;
            address lender = offer.lender;
            /// @dev Please note that we can now have many tranches with same `loanId`.
            tranche[i] = Tranche(loanId, totalAmount, amount, lender, 0, block.timestamp, offer.aprBps);
            totalAmount += amount;
@>          totalAmountWithMaxInterest += amount + amount.getInterest(offer.aprBps, _duration);
...

    function _checkOffer(
        LoanOffer calldata _offer,
        address _principalAddress,
        address _nftCollateralAddress,
        uint256 _amountWithInterestAhead
    ) private pure {
        if (_offer.principalAddress != _principalAddress || _offer.nftCollateralAddress != _nftCollateralAddress) {
            revert InvalidAddressesError();
        }
@>      if (_amountWithInterestAhead > _offer.maxSeniorRepayment) {
            revert InvalidTrancheError();
        }
    }

Note: totalAmountWithMaxInterest is computed using loan._duration

But when the bidding ends and the distribution is done LiquidationDistributor.distribute() the current time is used to calculate Interest.

    function distribute(uint256 _proceeds, IMultiSourceLoan.Loan calldata _loan) external {
        uint256[] memory owedPerTranche = new uint256[](_loan.tranche.length);
        uint256 totalPrincipalAndPaidInterestOwed = _loan.principalAmount;
        uint256 totalPendingInterestOwed = 0;
        for (uint256 i = 0; i < _loan.tranche.length;) {
            IMultiSourceLoan.Tranche calldata thisTranche = _loan.tranche[i];
            uint256 pendingInterest =
@>              thisTranche.principalAmount.getInterest(thisTranche.aprBps, block.timestamp - thisTranche.startTime);
            totalPrincipalAndPaidInterestOwed += thisTranche.accruedInterest;
            totalPendingInterestOwed += pendingInterest;
            owedPerTranche[i] += thisTranche.principalAmount + thisTranche.accruedInterest + pendingInterest;
            unchecked {
                ++i;
            }
        }

Because bidding takes a certain amount of time (3~7days), using block.timestamp - thisTranche.startTime will be larger than expected! Correctly should use: (loan.startTime + loan.duration - thisTranche.startTime) to calculate the interest.

This leads to the problem that if there is not enough funds, the front lender will get a larger repayment than expected, breaking the back lender's initial expectation of `maxSeniorRepayment

Impact

If there are not enough funds, the initial expectation of maxSeniorRepayment may be broken

Recommended Mitigation

    function distribute(uint256 _proceeds, IMultiSourceLoan.Loan calldata _loan) external {
        uint256[] memory owedPerTranche = new uint256[](_loan.tranche.length);
        uint256 totalPrincipalAndPaidInterestOwed = _loan.principalAmount;
        uint256 totalPendingInterestOwed = 0;
+      uint256 loanExpireTime = _loan.startTime + _loan.duration;
        for (uint256 i = 0; i < _loan.tranche.length;) {
            IMultiSourceLoan.Tranche calldata thisTranche = _loan.tranche[i];
            uint256 pendingInterest =
-               thisTranche.principalAmount.getInterest(thisTranche.aprBps, block.timestamp - thisTranche.startTime);
+               thisTranche.principalAmount.getInterest(thisTranche.aprBps, loanExpireTime - thisTranche.startTime);
            totalPrincipalAndPaidInterestOwed += thisTranche.accruedInterest;
            totalPendingInterestOwed += pendingInterest;
            owedPerTranche[i] += thisTranche.principalAmount + thisTranche.accruedInterest + pendingInterest;
            unchecked {
                ++i;
            }
        }

Assessed type

Context

c4-judge commented 7 months ago

0xA5DF marked the issue as primary issue

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

0xend commented 7 months ago

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