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

Partial Repayment Issue for Loans at Minimum Borrow Threshold in LendingTerm Contract #334

Closed
c4-bot-4 opened this issue Dec 22, 2023 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1182 edited-by-warden grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-4
Copy link
Contributor

c4-bot-4 commented Dec 22, 2023

Lines of code

https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/LendingTerm.sol#L528
https://github.com/volt-protocol/ethereum-credit-guild/blob/4d33abf95fee69391af0652e3cbe5e0cffa25f9f/src/loan/LendingTerm.sol#L529

Vulnerability details

Impact

The protocol allows partial repayments of debt on all open loans which has option to repay debt partially.
The feature of partial repayments is important because for correlated assets loan with interest building up may become insolvent. Since this issue do not allow borrowers with minimum borrow amount, to repay debt partially. Borrowers position may go underwater due to interest building up and bad debt may occur.

Proof of Code

Add the following test to the LendingTerm.t.sol

function testPartialRepaySuccessForMinBorrow() public {
        // prepare & borrow
        uint256 borrowAmount = 100e18;
        uint256 collateralAmount = 0.06e18;
        collateral.mint(address(this), collateralAmount);
        collateral.approve(address(term), collateralAmount);
        bytes32 loanId = term.borrow(borrowAmount, collateralAmount);
        assertEq(term.getLoan(loanId).collateralAmount, collateralAmount);

        // partialRepay
        vm.warp(block.timestamp + term.YEAR());
        vm.roll(block.number + 1);
        assertEq(term.getLoanDebt(loanId), 120e18);
        credit.mint(address(this), 60e18);
        credit.approve(address(term), 60e18);
        term.partialRepay(loanId, 60e18);

    }
  • Test includes successful borrowing the the minimum borrow amount and trying to partially repay loan with passing all other condition checks
  • The test will always revert as require( borrowAmount - issuanceDecrease > ProfitManager(refs.profitManager).minBorrow(), "LendingTerm: below min borrow" ); condition will never be passed for the loan with minimum borrow amount.
  • Therefore the borrowers with nearby minimum borrowed amount will not be able to repay loan partially.

Output for testPartialRepaySuccessForMinBorrow :

Running 1 test for test/unit/loan/LendingTerm.t.sol:LendingTermUnitTest
[FAIL. Reason: revert: LendingTerm: below min borrow] testPartialRepaySuccessForMinBorrow() (gas: 486964)
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 61.85ms

Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/unit/loan/LendingTerm.t.sol:LendingTermUnitTest
[FAIL. Reason: revert: LendingTerm: below min borrow] testPartialRepaySuccessForMinBorrow() (gas: 486964)

Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

  • Implement a special case in the contract to handle loans that are near the minimum borrow threshold, allowing them to be partially repaid down to the minimum borrow.

Assessed type

Other

@c4-bot-4 c4-bot-4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 22, 2023
c4-bot-3 added a commit that referenced this issue Dec 22, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Jan 2, 2024
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #1182

@c4-judge c4-judge removed the 3 (High Risk) Assets can be stolen/lost/compromised directly label Jan 28, 2024
@c4-judge
Copy link
Contributor

Trumpero changed the severity to QA (Quality Assurance)

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jan 28, 2024
@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-b

@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-c

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1182 edited-by-warden grade-c QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants