LendingTerm.debtCeiling is incorrectly limited by creditMinterBuffer, resulting in locked gauge weight

[c4-bot-2]

Lines of code

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

Vulnerability details

Impact

The debtCeiling() of LendingTerm's is limited by the creditMinterBuffer. Since creditMinterBuffer represents a remaining amount available to mint while debtCeiling represents an amount available to borrow + total issuance, limiting the debtCeiling to creditMinterBuffer may limit the debtCeiling much more than intended. This can result in some gauge weight from all gauges being locked, preventing gauge voters from decrementing their gauge weight since the debtCeiling will be too low.

Proof of Concept

RateLimitedMinter.buffer() returns the current amount of tokens available to be minted.

function buffer() public view returns (uint256) {
        uint256 elapsed = block.timestamp.safeCastTo32() - lastBufferUsedTime;
        return
                Math.min(bufferStored + (rateLimitPerSecond * elapsed), bufferCap);
}

The buffer is used in LendingTerm.debtCeiling to limit the debtCeiling to be no greater than the buffer. Of course, as mentioned, the problem with this is that the debtCeiling represents the current issuance + the amount of tokens available to mint, while the buffer only represents the amount of tokens available to mint.

When actually borrowing, users can borrow up to the buffer amount, which is as intended. This however, can result in a circumstance where the debtCeiling() returns an incorrect value which is less than the current issuance. In this circumstance, since gauge voters cannot decrement their gauge weight if it results in the debtCeiling < issuance, gauge voters may have their positions unexpectedly stuck.

// From GuildToken._decrementGaugeWeight
if (issuance != 0) {
    uint256 debtCeilingAfterDecrement = LendingTerm(gauge).debtCeiling(-int256(weight));
    require(
        issuance <= debtCeilingAfterDecrement,
        "GuildToken: debt ceiling used"
    );
}

Since the creditMinterBuffer is used throughout every gauge for the given credit, many gauge voters can be DoS'd at the same time.

Tools Used

• Manual review

Recommended Mitigation Steps

debtCeiling should be limited by creditMinterBuffer + issuance rather than just creditMinterBuffer.

Assessed type

Invalid Validation

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #335

[c4-judge]

Trumpero marked the issue as not a duplicate

[c4-judge]

Trumpero changed the severity to 2 (Med Risk)

[Trumpero]

This issue is invalid because it represents the correct behavior to ensure that debtCeiling is always lower than creditMinterBuffer, which guarantees that issuance is always lower than the buffer of credit tokens. Additionally, when the issuance of a lending term reaches the debtCeiling, gauge voters should be unable to decrement the gauge

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[kadenzipfel]

This is incorrectly marked as invalid.

This issue is invalid because it represents the correct behavior to ensure that debtCeiling is always lower than creditMinterBuffer, which guarantees that issuance is always lower than the buffer of credit tokens.

This doesn't guarantee that the issuance is always lower than the buffer of credit tokens, nor is it the intended effect.

debtCeiling includes the already issued tokens, representing an amount available to borrow + the already issued amount of tokens. Whereas creditMinterBuffer represents a remaining amount available to mint. So by limiting the debtCeiling to not be in excess of the creditMinterBuffer, we are enforcing that:

amount available to borrow + current issuance <= amount available to mint

This means that the total size of a lending term (debtCeiling) can never exceed the current amount available to mint (creditMinterBuffer). Clearly the intended effect here is that only the amount available to borrow is limited by the amount available to mint. This of course causes problems as lending terms grow larger while the creditMinterBuffer gets smaller, because once a lending term exceeds the creditMinterBuffer, gauge voters are unexpectedly unable to withdraw, meanwhile the creditMinterBuffer may continue to get smaller as smaller lending terms can still mint more tokens.

This is further exacerbated by the fact that, as described by #284, the rateLimitPerSecond is permanently stuck at 0.

[Trumpero]

@kadenzipfel Agree with you, the above statement is my mistake. I believe this issue is a dup of #868, and I missed this issue and my comment after judging #868.

[c4-judge]

Trumpero marked the issue as duplicate of #868

[c4-judge]

Trumpero marked the issue as satisfactory