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

0 stars 0 forks source link

Upgraded Q -> 3 from #35 [1713794205670] #87

Closed c4-judge closed 7 months ago

c4-judge commented 7 months ago

Judge has assessed an item in Issue #35 as 3 risk. The relevant finding follows:

Lines of code

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

Vulnerability details

Impact

The attackers make it impossible for borrowers to repay their debts, and the collateral is liquidated when the debts mature.

Proof of Concept

repayLoan, need to check the loanId, if the id is inconsistent will revert.

    function repayLoan(LoanRepaymentData calldata _repaymentData) external override nonReentrant {
        uint256 loanId = _repaymentData.data.loanId;
        Loan calldata loan = _repaymentData.loan;
        .....
@>      _baseLoanChecks(loanId, loan);
        .....
    }

    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 problem is that _loans[_loanId] can change, for example, when mergeTranches delete the old loanId and write the new one.

    _loans[loanId] = loanMergedTranches.hash();
    delete _loans[_loanId];

An attacker can use the front-running attack method, when repayLoan is called, execute the mergeTranches function in advance, and make the id in _loans updated. In this case, the repayLoan execution will fail due to inconsistent _loanId.

If the attacker keeps using this attack, the borrower's debt will not be repaid, eventually causing the collateral to be liquidated.

In addition to the mergeTranches function, the attacker can also call addNewTranche, and the borrower can also call the refinance-related function, again causing _loanId to be updated.

An attacker can also use the same method to attack refinance related functions, making refinance unable to execute.

An attacker can also use the same method to attack the liquidateLoan function, making it impossible for debts to be cleared.

Tools Used

vscode, manual

Recommended Mitigation Steps

Do not delete _loanId

Assessed type

DoS

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

c4-judge commented 7 months ago

0xA5DF marked the issue as nullified