Ethereum Credit Guild - Topmark's results

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/governance/LendingTermOnboarding.sol#L128 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L545-L553 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L595-L607

Vulnerability details

Impact

Users would be unable to perform Partial Loan Repayment whenever Interest rate is Zero

Proof of Concept

    /// @notice Create a new LendingTerm and initialize it.
    function createTerm(
        address implementation,
        LendingTerm.LendingTermParams calldata params
    ) external returns (address) {
...
require(
            params.interestRate < 1e18, // interest rate [0, 100[% APR
            "LendingTermOnboarding: invalid interestRate"
        );
  ...
}

The function above shows the origin of Loan Terms and how loan conditions are set in the LendingTermOnboarding.sol contract, the point of interest is how interest Rate is set, as noted in it comment description interest rate can indeed be 0 percent to 100 percent, 0% including.

function _repay(address repayer, bytes32 loanId) internal {
        ...
>>>        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)
            );
        }
...

The implication of this can be seen in how repay loan function is set in the LendingTerm.sol contract, from the repayment of Loan function above , it can be noted that interest of zero is put into consideration to avoid denial of service where necessary.

function _partialRepay(
        address repayer,
        bytes32 loanId,
        uint256 debtToRepay
    ) internal {
      ...
        require(
>>>            principalRepaid != 0 && interestRepaid != 0,
            "LendingTerm: repay too small"
        );
       ...
        // forward profit portion to the ProfitManager, burn the rest
        CreditToken(refs.creditToken).transfer(
            refs.profitManager,
            interestRepaid
        );
        ProfitManager(refs.profitManager).notifyPnL(
            address(this),
            int256(interestRepaid)
        );
...

However that is not the case in the partial repayment function as "interestRepaid" is required not to be zero and no consideration is done in event "interestRepaid" is zero, causing Denial of Service to Borrowers who don't have the ability to repay all loan at once which may cause them to unfairly loss their collateral when "maxDelayBetweenPartialRepay" timer runs out.

Tools Used

Manual Review

Recommended Mitigation Steps

Necessary validation should be adjusted and added into the _partialRepay(...) function before transfer of interest is attempted to avoid reversion as provided below

function _partialRepay(
        address repayer,
        bytes32 loanId,
        uint256 debtToRepay
    ) internal {
      ...
        require(
---            principalRepaid != 0 && interestRepaid != 0,
+++            principalRepaid != 0 ,
            "LendingTerm: repay too small"
        );
       ...
+++         if (interestRepaid != 0) {
        // forward profit portion to the ProfitManager, burn the rest
        CreditToken(refs.creditToken).transfer(
            refs.profitManager,
            interestRepaid
        );
        ProfitManager(refs.profitManager).notifyPnL(
            address(this),
            int256(interestRepaid)
        );
+++  }
...

Assessed type

DoS