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

No liquidation mechanism is present for non-periodic payment loans. #1174

Closed
c4-bot-7 opened this issue Dec 28, 2023 · 4 comments
Closed

No liquidation mechanism is present for non-periodic payment loans. #1174

c4-bot-7 opened this issue Dec 28, 2023 · 4 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 duplicate-1057 satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-7
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/loan/LendingTerm.sol#L652-L656

Vulnerability details

Impact

Liquidations are an important mechanism to keep a lending platform solvent. This platform enables liquidations using an auction mechanism to ensure best prices for defaulted accounts. This is handled via the call function in LendingTerm.sol contract which checks if the loan is indeed able to be auctioned off in the following snippet.

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

The issue is that the partialRepayDelayPassed always returns false for non-periodic payment loans. The system tracks the value maxDelayBetweenPartialRepay which ensures regular payments are made. The system also shows intention of supporting loans which dont have periodic payments, by setting this value to 0 as described in the comments.

/// @notice maximum delay, in seconds, between partial debt repayments.
/// if set to 0, no periodic partial repayments are expected.
/// if a partial repayment is missed (delay has passed), the loan
/// can be called.
uint256 maxDelayBetweenPartialRepay;

The issue is that if maxDelayPartialRepay is set to 0, the partialRepayDelayPassed function will always return false as seen in the following snippet.

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

So the only way to liquidate loans without payment plans is to deprecate the entire gauge. This is not ideal as it would liquidate all loans in the gauge. This is a design issue that can lead to insolvency of the protocol.

Proof of Concept

This is a design issue and basically has missing code to handle loans which the documentation in the code claims to support. No POC is provided since the functionality is entirely missing.

Tools Used

Manual review

Recommended Mitigation Steps

Implement a due date mechanism for loans without payment plans. If the due date passes, the loan will be eligible for liquidation.

Assessed type

Other

@c4-bot-7 c4-bot-7 added 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 labels Dec 28, 2023
c4-bot-4 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 sufficient quality report This report is of sufficient quality primary issue Highest quality submission among a set of duplicates labels Dec 30, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as primary issue

@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 the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 26, 2024
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 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