jkoppel - Calling Cooler.claimDefaulted will lose all unclaimed payments.

[sherlock-admin]

jkoppel

medium

Calling Cooler.claimDefaulted will lose all unclaimed payments.

Summary

Loans not using repayDirect (repayDirect = false) must call Cooler.claimRepaid to get their unclaimed debt payments.
However, if someone calls Cooler.claimDefaulted first, loan.unclaimed is set to 0 wiping any ability to claim them.

Vulnerability Detail

1. Alice calls Cooler.clearRequest with repayDirect set to false
2. Bob, who made the loanRequest pays back the loan in time.
3. The time for the loan runs out
4. Charles calls Cooler.claimDefaulted on that loan
5. Alice calls claimRepaid and gets zero tokens.

Note that it is possible to call both Cooler.claimDefaulted and Clearinghouse.claimDefaulted on loans that have been fully repaid.

Impact

Lenders using repayDirect = false will likely lose all debt payments.
Two likely scenarios

1. Defaulted loans that have received some payments
   The lender will only get the remaining collateral and not any payments (if they don't call claimRepaid first).
2. Loans that are paid back, but the lender does not claim the payments before the expiry date.
   In this scenario, the lender will lose all payments

Even without malicious intent, unclaimed loans will be wiped when someone calls ClearingHouse.claimDefaulted, which is encouraged.

Code Snippet

https://github.com/sherlock-audit/2023-08-cooler/blob/main/Cooler/src/Cooler.sol#L315-L333

Cooler.claimDefaulted

    /// @notice Claim collateral upon loan default.
    /// @param loanID_ index of loan in loans[]
    /// @return defaulted debt by the borrower, collateral kept by the lender, elapsed time since expiry.
    function claimDefaulted(uint256 loanID_) external returns (uint256, uint256, uint256) {
        Loan memory loan = loans[loanID_];
        delete loans[loanID_]; //<--- @audit removes all information on loan

        if (block.timestamp <= loan.expiry) revert NoDefault();

        // Transfer defaulted collateral to the lender.
        collateral().safeTransfer(loan.lender, loan.collateral); // <---- @audit only loan.collateral, loan.unclaimed can be non-0 at this point

        // Log the event.
        factory().newEvent(loanID_, CoolerFactory.Events.DefaultLoan, 0);

        // If necessary, trigger lender callback.
        if (loan.callback) CoolerCallback(loan.lender).onDefault(loanID_, loan.amount, loan.collateral);
        return (loan.amount, loan.collateral, block.timestamp - loan.expiry);
    }

https://github.com/sherlock-audit/2023-08-cooler/blob/main/Cooler/src/Cooler.sol#L302-L313

    /// @notice Claim debt tokens if repayDirect was false.
    /// @param  loanID_ index of loan in loans[].
    function claimRepaid(uint256 loanID_) external {
        Loan memory loan = loans[loanID_];

        // Update storage.
        uint256 claim = loan.unclaimed; // <--- @audit This value is always zero after claimDefaulted has been called successfully. 
        delete loans[loanID_].unclaimed;

        // Transfer repaid debt back to the lender.
        debt().safeTransfer(loan.lender, claim);
    }

Tool used

Manual Review

Recommendation

Cooler.claimDefaulted should send along loan.unclaimed before wiping the loan.

Duplicate of #119