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

9 stars 5 forks source link

Inability to unstake if the credit minter buffer is low #1247

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 6 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L226-L230

Vulnerability details

Impact

The LendingTerm.debtCeiling() function, only called from GuildToken._decrementGaugeWeight(), which is itself internally called when unstaking/decreasing gauge weight, contains a mistake in calculation that will prevent users from unstaking from any term if the creditMinterBuffer is low. This is because it will always return the creditMinterBuffer if it is lower than the _hardCap or _debtCeiling. The only exception is the case where _issuance >= debtCeilingBefore, in which case debtCeilingBefore will be returned, which will then fail the require(issuance <= debtCeilingAfterDecrement) statement in _decrementGaugeWeight() unless in case of strict equality.

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L290-L291 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L302-L303 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L309-L311 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L316-L317 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/LendingTerm.sol#L323-L330 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/GuildToken.sol#L226-L230

It seems that this function was written with another purpose in mind (to limit minting) and then repurposed to fulfil another purpose (to determine how much can be unstaked).

This could lead to a situation where, under heavy protocol usage or simply after a large mint, users are unable to unstake or decrease their gauge weight, regardless of the issuance on the term they're supporting.

Proof of Concept

Consider the following scenario:

  1. The protocol experiences high usage, reducing the creditMinterBuffer.
  2. A user tries to unstake or decrease their gauge weight from a term they're supporting, which is far from reaching its debt ceiling.
  3. The GuildToken._decrementGaugeWeight() function is called, which internally calls the debtCeiling() function.
  4. The debtCeiling() function returns the creditMinterBuffer as it is lower than the _hardCap or _debtCeiling.
  5. The user is unable to unstake or decrease their gauge weight as the creditMinterBuffer is lower than the term's issuance.

Additionally, note that this also opens up a DoS vector (albeit probably beneficial to the protocol) exploitable by continually minting CREDIT and depleting the minting buffer, which will prevent anybody from unstaking or reducing their weight.

Tools Used

Manual review

Recommended Mitigation Steps

The debtCeiling() function needs to be re-architected to ensure it only prevents unstaking based on a term's issuance.

Assessed type

Other

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

0xSorryNotSorry marked the issue as duplicate of #878

c4-judge commented 5 months ago

Trumpero changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

Trumpero marked the issue as grade-b

c4-judge commented 5 months ago

This previously downgraded issue has been upgraded by Trumpero

c4-judge commented 5 months ago

Trumpero changed the severity to QA (Quality Assurance)

c4-judge commented 5 months ago

Trumpero removed the grade

c4-judge commented 5 months ago

Trumpero marked the issue as grade-b

Trumpero commented 4 months ago

I consider this issue as a dup of https://github.com/code-423n4/2023-12-ethereumcreditguild-findings/issues/868 since it shares the same idea about the problem, where capping debtCeiling with creditMinterBuffer causes the debtCeiling to be lower than it should.

thebrittfactor commented 4 months ago

For transparency, the judge has requested for the labels on this submission to be updated.

c4-judge commented 4 months ago

Trumpero marked the issue as satisfactory