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

17 stars 11 forks source link

Ensure that repayment process of the _partialRepay function in the LendingTerm contract. #71

Closed c4-bot-1 closed 9 months ago

c4-bot-1 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L490

Vulnerability details

Impact

Consider these changes to ensure that the contract's state accurately reflects its intended logic, especially concerning loans' statuses and financial parameters. Hypothetically, an attacker might attempt to exploit the system by manipulating the partial repayment process though the _partialRepay function in the LendingTerm contract.

Mitigation Strategies: To mitigate such attacks, the following security measures can be implemented in the _partialRepay function:

  1. Enforce Minimum Repayment Percentages: Ensure that each partial repayment meets a certain minimum percentage of the total debt, preventing exploitatively small repayments.

  2. Regular Loan Status Checks: Continuously verify that the loan is open and not called before allowing a partial repayment, ensuring the loan's state is appropriate for such actions.

  3. Accurate Debt Update and Validation: After each partial repayment, accurately update the loan's remaining amount and validate the changes to maintain the integrity of the loan's financial data.

  4. Time-Based Restrictions or Penalties: Implement mechanisms that discourage timing manipulation, like penalties for delayed repayments or incentives for timely repayments within certain windows.

Proof of Concept

Imagine a hypothetical attack scenario targeting the _partialRepay function in the LendingTerm contract, an attacker might attempt to exploit the system by manipulating the partial repayment process. Here's how such an attack could unfold:

Scenario Setup Attacker's Objective: The attacker aims to disrupt the loan system's financial balance, either by avoiding due penalties, gaining undue advantages, or causing losses to other participants.

Steps of the Attack Observation and Planning: The attacker carefully observes the functioning of the _partialRepay function, identifying potential weaknesses or loopholes in the repayment logic, such as the ability to make extremely small repayments or timing the repayments to avoid penalties.

Acquiring a Loan: The attacker takes out a loan from the LendingTerm contract, ensuring they have a position from which to execute their plan.

Exploiting Partial Repayment Mechanics:

  1. Minimal Repayment: The attacker might attempt to make a series of minimal repayments, far below the reasonable or expected amount, with the goal of avoiding larger interest accruals or delaying the full repayment without facing significant penalties.

  2. Timing Manipulation: If the contract has certain time-based penalties or benefits associated with repayments, the attacker could time their repayments to avoid penalties or maximize benefits unjustly.

  3. Repeated Execution: The attacker continues this pattern of exploitation, either on the same loan or across multiple loans, to continually benefit at the expense of the system's intended financial balance.

Potential System Disruption: Such actions, especially if replicated by multiple users, could lead to a significant imbalance in the loan system. It might cause financial strain on the system, unfair advantages to certain users, and potentially undermine the trust in the platform.

To implement security checks in the _partialRepay function of the provided Solidity smart contract, we need to focus on confirming that the loan is open and not called, enforcing the minimum partial repayment percentages, and ensuring accurate updates to the loan's debt after repayment. Here's an updated version of the _partialRepay function with the necessary security checks:

function _partialRepay( address repayer, bytes32 loanId, uint256 debtToRepay ) internal { Loan storage loan = loans[loanId];

// Check the loan is open and not called
uint256 borrowTime = loan.borrowTime;
require(borrowTime != 0, "LendingTerm: loan not found");
require(borrowTime < block.timestamp, "LendingTerm: loan opened in same block");
require(loan.closeTime == 0, "LendingTerm: loan closed");
require(loan.callTime == 0, "LendingTerm: loan called");

// Compute partial repayment
uint256 loanDebt = getLoanDebt(loanId);
require(debtToRepay < loanDebt, "LendingTerm: full repayment");
uint256 percentRepaid = (debtToRepay * 1e18) / loanDebt; // [0, 1e18[
uint256 borrowAmount = loan.borrowAmount;
uint256 creditMultiplier = ProfitManager(refs.profitManager).creditMultiplier();
uint256 principal = (borrowAmount * loan.borrowCreditMultiplier) / creditMultiplier;
uint256 principalRepaid = (principal * percentRepaid) / 1e18;
uint256 interestRepaid = debtToRepay - principalRepaid;
uint256 issuanceDecrease = (borrowAmount * percentRepaid) / 1e18;

// Enforce minimum partial repayment percentages
require(
    principalRepaid != 0 && interestRepaid != 0,
    "LendingTerm: repay too small"
);
require(
    debtToRepay >= (loanDebt * params.minPartialRepayPercent) / 1e18,
    "LendingTerm: repay below min"
);
require(
    borrowAmount - issuanceDecrease > ProfitManager(refs.profitManager).minBorrow(),
    "LendingTerm: below min borrow"
);

// Update loan in state and ensure accurate debt update
loans[loanId].borrowAmount -= issuanceDecrease;
lastPartialRepay[loanId] = block.timestamp;
issuance -= issuanceDecrease;

// Handle transfer and burning of CREDIT tokens
CreditToken(refs.creditToken).transferFrom(repayer, address(this), debtToRepay);
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);

} Changes Made and Why:

  1. Loan Status Checks: Added checks to ensure the loan is open (not closed or called) before allowing a partial repayment. This prevents repayments on invalid or already settled loans.

  2. Minimum Repayment Enforcement: Added a check to ensure the repayment is not too small and adheres to the minimum partial repayment percentage set in the contract. This prevents negligible repayments that could exploit the system.

  3. Accurate Debt Update: Ensured the loan's debt is accurately updated post-repayment, reflecting the actual amount repaid. This is crucial for maintaining the integrity of the loan and its terms.

  4. Event Emission: Maintained the emission of the LoanPartialRepay event to ensure transparency and traceability of transactions within the smart contract.

Tools Used

Recommended Mitigation Steps

In the _partialRepay function of your Solidity smart contract, I introduced several key changes to enhance security and ensure the function operates as intended:

  1. Loan Open and Not Called Check: Added checks to verify that the loan is currently open and has not been called. This is crucial to prevent repayments on loans that are either already closed or in a state where they shouldn't be subject to partial repayments (like being in an auction process after a call).

  2. Repayment Amount Validity Check: Included a condition to ensure that the debt being repaid is less than the total loan debt, which prevents full repayment through this function. The intention is to differentiate between partial and full repayments and handle them through their respective functions.

  3. Minimum Repayment Threshold Enforcement: Added a check to ensure that the amount being repaid adheres to the minimum partial repayment percentage set in the contract. This is to prevent exploitatively small repayments that could disrupt the loan's intended financial structure.

  4. Loan Data Updates and Integrity Check: Ensured that the loan's remaining amount and the system's issuance are correctly updated after the repayment. This is critical for maintaining the integrity of the loan and ensuring that the system's records accurately reflect the current state of each loan.

  5. Transfer and Burning of CREDIT Tokens: Managed the transfer and burning of CREDIT tokens to handle the repayment. This includes transferring the interest portion to the ProfitManager and burning the principal, which is a critical step in the repayment process, affecting the overall token supply and distribution of rewards.

  6. Event Logging: Maintained the emission of the LoanPartialRepay event. This is important for transparency, allowing external observers (like interfaces or other contracts) to track the occurrence and details of partial repayments.

Assessed type

Access Control

c4-bot-8 commented 9 months ago

Withdrawn by AerialRaider