DOS all Pool's offer through capacity=0

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/loans/MultiSourceLoan.sol#L124

Vulnerability details

Vulnerability details

If offer.capacity=0, then this offer.offerId becomes one-time.

emitLoan()->_processOffersFromExecutionData()

    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) {
...

            _handleProtocolFeeForFee(
                offer.principalAddress, lender, fee.mulDivUp(protocolFee.fraction, _PRECISION), protocolFee.recipient
            );

            ERC20(offer.principalAddress).safeTransferFrom(lender, _principalReceiver, amount - fee);
            if (offer.capacity > 0) {
                _used[lender][offer.offerId] += amount;
            } else {
@>              isOfferCancelled[lender][offer.offerId] = true;
            }

            offerIds[i] = offer.offerId;
            unchecked {
                ++i;
            }
        }
        Loan memory loan = Loan(
            _borrower,
            _tokenId,
            _nftCollateralAddress,
            _principalAddress,
            totalAmount,
            block.timestamp,
            _duration,
            tranche,
            protocolFee.fraction
        );

        return (loanId, offerIds, loan, totalFee);
    }

This gives a malicious attacker an opportunity to maliciously attack all offers with lender == Pool. Example Bob call emitLoan(lender == Pool, offerId = 123)

1. Alice front-run call emitLoan(lender == Pool, offerId = 123,capacity=0, duration=0)
   • after execute , isOfferCancelled[Pool][123] = true
2. Bob's tranaction will fail, because isOfferCancelled[Pool][123] = true;
3. Alice call repayLoan() get back nft

Impact

DOS all Pool's offer

Recommended Mitigation

If lender is LoanManager, then offer.capacity must not be 0.

    function _validateOfferExecution(
        OfferExecution calldata _offerExecution,
        uint256 _tokenId,
        address _lender,
        address _offerer,
        bytes calldata _lenderOfferSignature,
        uint256 _feeFraction,
        uint256 _totalAmount
    ) private {
...

+      if (getLoanManagerRegistry.isLoanManager(_lender) && offer.capacity==0) {
+          revert InvalidTrancheError();
+      }

       if ((offer.capacity > 0) && (_used[_offerer][offer.offerId] + _offerExecution.amount > offer.capacity)) { 
            revert MaxCapacityExceededError();
        }

        _checkValidators(_offerExecution.offer, _tokenId);
    }

Assessed type

DoS

[c4-judge]

0xA5DF marked the issue as primary issue

[0xA5DF]

Sponsor: Please comment on the severity of the issue If issuing a new offer is a no brainer then this falls under this category - https://github.com/code-423n4/org/issues/143

[0xend]

It does fall within that category. Attacker would need to be emitting loans + repaying loans to constantly DoS.

[c4-judge]

0xA5DF changed the severity to QA (Quality Assurance)

[0xA5DF]

Marking as low, open to hear arguments for med

[c4-judge]

0xA5DF marked the issue as grade-c

[0xA5DF]

Moved to #74