c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L486-L630

Vulnerability details

Proof of Concept

Take a look at both functions from LendingTerm.sol#L486-L630


    function _repay(address repayer, bytes32 loanId) internal {
        Loan storage loan = loans[loanId];
//...ommited for brevity

    /// pull debt from the borrower and replenish the buffer of available debt that can be minted.
        CreditToken(refs.creditToken).transferFrom(
            repayer,
            address(this),
            loanDebt
        );
        if (interest != 0) {
            // forward profit portion to the ProfitManager
            CreditToken(refs.creditToken).transfer(
                refs.profitManager,
                interest
            );

            // report profit
            ProfitManager(refs.profitManager).notifyPnL(
                address(this),
                int256(interest)
            );
        }

        // burn loan principal
        CreditToken(refs.creditToken).burn(principal);
        RateLimitedMinter(refs.creditMinter).replenishBuffer(principal);

        // close the loan
        loan.closeTime = block.timestamp;
        issuance -= borrowAmount;

        // return the collateral to the borrower
        //@audit this is not present in partialrepay() 
        IERC20(params.collateralToken).safeTransfer(
            loan.borrower,
            loan.collateralAmount
        );

        // emit event
        emit LoanClose(block.timestamp, loanId, LoanCloseType.Repay, loanDebt);
    }

    function _partialRepay(
        address repayer,
        bytes32 loanId,
        uint256 debtToRepay
    ) internal {
        Loan storage loan = loans[loanId];
//...ommited for brevity
  // pull the debt from the borrower
        CreditToken(refs.creditToken).transferFrom(
            repayer,
            address(this),
            debtToRepay
        );

        // forward profit portion to the ProfitManager, burn the rest
        CreditToken(refs.creditToken).transfer(
            refs.profitManager,
            interestRepaid
        );
        ProfitManager(refs.profitManager).notifyPnL(
            address(this),
            int256(interestRepaid)
        );
        CreditToken(refs.creditToken).burn(principalRepaid);
        RateLimitedMinter(refs.creditMinter).replenishBuffer(principalRepaid);

        // emit event
        emit LoanPartialRepay(block.timestamp, loanId, repayer, debtToRepay);

}

As seen logic of both functions are very similar with the only difference being that _partialRepay() doesn't include a full repaty and as such the loan doesn't get closed.

Also evidently, we can see that both functions take the same steps from pulling the debt from the borrower to replenish the buffer, then forwarding the profit portion to the manager and calling notifyPnL() on the manager even burning the amount of principal that was provided, issue now is that where as repay() includes the logic of transferring the collateral to the borrower, partialRepay() does not which leaves the borrower not to receive the partial collateral that's been cleared

Impact

Collateral is not being returned to the borrower when partially repayed via LedingTerm#partialRepay()

Recommended Mitigation Steps

Since the loan can't be closed cause not all necessary principal is being accounted for in partialRepay() send the borrower the amount of collateral that matches the principalRepaid that was provided.

Assessed type

Token-Transfer

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as duplicate of #783

c4-judge commented 5 months ago

Trumpero marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-12-ethereumcreditguild-findings