_baseLoanChecks() check errors for expire

criticalCode4rena

Lines of code

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

Vulnerability details

Vulnerability details

_baseLoanChecks() is used to check whether Loan has expired.

solidity📋
    function _baseLoanChecks(uint256 _loanId, Loan memory _loan) private view {
        if (_loan.hash() != _loans[_loanId]) {
            revert InvalidLoanError(_loanId);
        }
@>      if (_loan.startTime + _loan.duration < block.timestamp) {
            revert LoanExpiredError();
        }
    }

The expiration checks in liquidation are as follows:

solidity📋
    function _liquidateLoan(uint256 _loanId, IMultiSourceLoan.Loan calldata _loan, bool _canClaim)
        internal
        returns (bool liquidated, bytes memory liquidation)
    {
...

        uint256 expirationTime = _loan.startTime + _loan.duration;
@>      if (expirationTime > block.timestamp) {
            revert LoanNotDueError(expirationTime);
        }

This way, both checks pass when block.timestamp == _loan.startTime + _loan.duration

This leads to the problem that a malicious attacker can perform the following step when block.timestamp == _loan.startTime + _loan.duration

1. Alice call liquidateLoan(loandId =1) -> success
   • LoanLiquidator generates an auction
   • _loans[loandId = 1] is still valid , and will only be cleared when the auction is over.
2. Alice call addNewTranche(loandId = 1) -> success
   • _baseLoanChecks(loandId = 1) will pass
   • delete _loans[1];
   • _loans[2] = newLoan.hash()
3. bidding ends, call loanLiquidated(loandId = 1) will fail , because _loans[1] has been cleared

Impact

Maliciously disrupting the end of the bidding, causing the NFT/funds to be locked

Recommended Mitigation

diff📋

    function _baseLoanChecks(uint256 _loanId, Loan memory _loan) private view {
        if (_loan.hash() != _loans[_loanId]) {
            revert InvalidLoanError(_loanId);
        }
-       if (_loan.startTime + _loan.duration < block.timestamp) {
+      if (_loan.startTime + _loan.duration <= block.timestamp) {
            revert LoanExpiredError();
        }
    }

Assessed type

Context