c4-bot-4 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L323-L330

Vulnerability details

Description

The LendingTerm::debtCeiling() function calculates the min of creditMinterBuffer, _debtCeiling and _hardCap as shown below:

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

However, the above minimum logic is flawed, as it does not always return the minimum of the 3 values.

Impact

As the min() calculation is not correct, the LendingTerm::debtCeiling() might return the incorrect value, and so might return a higher debt ceiling rather than the actual debt ceiling as the function should be returning.

LendingTerm::debtCeiling() is used in GuildToken::_decrementGaugeWeight(), which will will make this function incorrect as well.

Proof of concept

If creditMinterBuffer was 3, _debtCeiling was 5, and _hardCap was 1, then the min of the 3 values should be _hardCap which is 1.

But instead, this condition becomes true creditMinterBuffer < _debtCeiling, which then returns creditMinterBuffer, which is incorrect.

Severity Justification

This is Medium severity, based on the Code4rena Severity Categorization: https://docs.code4rena.com/awarding/judging-criteria/severity-categorization

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Tools Used

Manual review

Recommended Mitigation Steps

Update the min() logic to be correct:

-   if (creditMinterBuffer < _debtCeiling) {
-      return creditMinterBuffer;
-   }
-   if (_hardCap < _debtCeiling) {
-      return _hardCap;
-   }
-   return _debtCeiling;
+   if (creditMinterBuffer < _debtCeiling && creditMinterBuffer < _hardCap) {
+       return creditMinterBuffer;
+   } else if (_debtCeiling < _hardCap) {
+       return _debtCeiling;
+   } else {
+       return _hardCap;
+   }

Assessed type

Other

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 9 months ago

0xSorryNotSorry marked the issue as primary issue

c4-sponsor commented 9 months ago

eswak (sponsor) confirmed

eswak commented 9 months ago

Very clear, thank you 👍

c4-judge commented 8 months ago

Trumpero marked the issue as satisfactory