code-423n4 / 2023-12-ethereumcreditguild-findings

9 stars 5 forks source link

Upgraded Q -> 2 from #765 [1706657343488] #1285

Closed c4-judge closed 5 months ago

c4-judge commented 5 months ago

Judge has assessed an item in Issue #765 as 2 risk. The relevant finding follows:

L-03

The LendingTerm.debtCeiling() function returns the debt ceiling for the given term. Unfortunately, the code is designed such that there is an off-chance that if _hardCap < creditMinterBuffer < (_issuance + maxBorrow), then the creditMintBuffer will be returned. In this case, _hardCap should be returned.

This can cause GuildToken._decrementGaugeWeight to decrement when it should if the above edge case occurs and the issuance is greater than the _hardCap.

The LendingTerm.debtCeiling() runs a series of calculations to determine the Term's debt ceiling. The debt ceiling should always lean towards returning the smallest acceptable value to ensure protocol safety and prevent runaway borrowing. You can see this safety check occur throughout the function where in most scenarios the debtCeiling() returns the following logical statement:

_hardCap < creditMinterBuffer ? _hardCap : creditMinterBuffer;

Unfortunately, at the end of the function the final set of code logic runs:

// return min(creditMinterBuffer, hardCap, debtCeiling)
if (creditMinterBuffer < _debtCeiling) {
    return creditMinterBuffer;
}
if (_hardCap < _debtCeiling) {
    return _hardCap;
}
return _debtCeiling;

This logic dictates that as long as creditMinterBuffer is lower than _debtCeiling, then creditMinterBuffer will be returned in the function. However, it does not consider if _hardCap is lower than the creditMinterBuffer. This can lead to a scenario where the debtCeiling() returns a value larger than expected.

We can see this in action in the following sample code below:

function debtCeiling() public returns (uint) {
    uint creditMinterBuffer = 100;
    uint _debtCeiling = 101;
    uint _hardCap = 99;

    uint returnValue;

    if (creditMinterBuffer < _debtCeiling) {
        return creditMinterBuffer;
    }
    if (_hardCap < _debtCeiling) {
        return _hardCap;
    }
}

uint ceiling = debtCeiling(); // will return 100

The protocol should re-work the function to do the following:

uint ceiling = _hardCap;

if (creditMinterBuffer < ceiling) {
    ceiling = creditMinterBuffer;
}
if (_debtCeiling < ceiling) {
    ceiling = _debtCeiling;
}

return ceiling;
c4-judge commented 5 months ago

Trumpero marked the issue as duplicate of #708

c4-judge commented 5 months ago

Trumpero marked the issue as satisfactory