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

For Loan Terms with No partial repay (maxDelayBetweenPartialRepay = 0), there is no way to liquidate the loan #921

Closed
c4-bot-10 opened this issue Dec 28, 2023 · 5 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1057 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-10
Copy link
Contributor

Lines of code

https://github.com/volt-protocol/ethereum-credit-guild/blob/main/src/loan/LendingTerm.sol#L634-L675

Vulnerability details

Impact

There is no way to liquidate the loan for terms with no partial repayment (maxDelayBetweenPartialRepay = 0).

Hence, it results in either of two things:

  1. Bad debt is collected
  2. The term is offboarded resulting in healthy loans to get liquidated.

Proof of Concept

https://github.com/volt-protocol/ethereum-credit-guild/blob/main/src/loan/LendingTerm.sol#L634-L675

require(
            GuildToken(refs.guildToken).isDeprecatedGauge(address(this)) ||
                partialRepayDelayPassed(loanId),
            "LendingTerm: cannot call"
);

The call function only allows calling loans based on two conditions:

  1. The gauge has been deprecated (generally the case with off-boarding term)
  2. The delay after the last re-payment has passed maxDelayBetweenPartialRepay

But if the maxDelayBetweenPartialRepay is set to 0, the partialRepayDelayPassed function always returns false, which results in the call function getting reverted as well.
https://github.com/volt-protocol/ethereum-credit-guild/blob/main/src/loan/LendingTerm.sol#L240-L241

function partialRepayDelayPassed(
        bytes32 loanId
    ) public view returns (bool) {
        // if no periodic partial repays are expected, always return false
        if (params.maxDelayBetweenPartialRepay == 0) return false;

        // if loan doesn't exist, return false
        if (loans[loanId].borrowTime == 0) return false;

        // if loan is closed, return false
        if (loans[loanId].closeTime != 0) return false;

        // return true if delay is passed
        return
            lastPartialRepay[loanId] <
            block.timestamp - params.maxDelayBetweenPartialRepay;
    }

Hence liquidation never happens for the loans in terms with maxDelayBetweenPartialRepay set to 0.

Tools Used

Manual

Recommended Mitigation Steps

There should be a way to liquidate specific loans in terms where maxDelayBetweenPartialRepay is set to 0.

Assessed type

Other

@c4-bot-10 c4-bot-10 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Dec 28, 2023
c4-bot-10 added a commit that referenced this issue Dec 28, 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 #1174

@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #1057

@c4-judge
Copy link
Contributor

Trumpero marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jan 26, 2024
@c4-judge
Copy link
Contributor

Trumpero changed the severity to 2 (Med Risk)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-1057 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants