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

17 stars 11 forks source link

No progression state can be easily reached #250

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 9 months ago

Lines of code

(https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/loan/SurplusGuildMinter.sol#L249-L252

Vulnerability details

Impact

The accrual of bad debt gives lenders an advantage in governance.

Proof of Concept

The documentation details that GuildVetoGovernor will be deployed twice (or more). The first deployment allows voting with guild tokens, and the second allows voting with credit tokens. This gives lenders have some influence over the markets, albeit limited to vetoing proposals.

When bad debt is accrued, the creditMultiplier decreases. This reduction enables the minting of more credit tokens using PSM. The ratio of guild to credit tokens worsens, giving lenders more power to veto proposals. For instance, a decrease in the creditMultiplier might lead voters to propose reducing the rewardRatio in a market. Lenders, having more power after the debt accrual, may choose to veto such proposals.

Example:

  1. Bad debt accrues, and creditMultiplier falls from 1e18 to 0.25e18.
  2. Due to the decreased creditMultiplier, PSM now mints four times the credits for the same amount of pegToken.
  3. After bad debt accrual, SGM stakers stake four times as many credits (with the same USD value as before the bad debt) and receive four times the guild rewards (rewardRatio unchanged).
  4. Voters notice this and propose a 4x reduction in rewardRatio.
  5. Lenders veto the proposal, as it will decrease their yield.

This scenario, though simple, illustrates how a drop in the CM ratio may disproportionately empower one side of the voting process.

Tools Used

Manual review.

Recommended Mitigation Steps

Updating rewardRatio in accordance with changes in mintRatio is recommended. Plus this avoids any unnecessary proposals to change the reward ratio.

Below is a basic example of how this can be achieved (code from SGM):

if (deltaIndex != 0) {
    uint256 creditReward = (uint256(userStake.guild) * deltaIndex) / 1e18;
-   uint256 guildReward = (creditReward * rewardRatio) / 1e18;

+   uint256 creditMultiplier = ProfitManager(profitManager).creditMultiplier();
+   uint256 guildRewardRatio = (creditReward * rewardRatio) / 1e18;
+   uint256 guildReward = (guildRewardRatio * creditMultiplier) / 1e18;  

    if (slashed) {
        guildReward = 0;
    }

Assessed type

Governance

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) acknowledged

c4-sponsor commented 9 months ago

eswak marked the issue as disagree with severity

eswak commented 9 months ago

I would flag this as informational, as this details a possible governance situation, and not an issue with the code. The issue states :

Lenders veto the proposal, as it will decrease their yield.

I don't believe there is a reason for lenders to veto this proposal, as it would not decrease the yield of the lenders.

SGM stakers who get slashed also probably won't re-stake overnight, or a least not in the same way, they may have learned a lesson...

A 4x creditMultiplier overnight would mean 75% of the protocol is bad debt and that is pretty unlikely. Governance is expected to regularly step in to adjust the protocol parameters and such big drifts overnight are not expected.

SGM stakers can only use the GUILD minted to participate in the gauge system, so it would be a different set of people (or at least a different set of 'token balances') that participate in governance / veto proposals.

c4-judge commented 8 months ago

Trumpero changed the severity to QA (Quality Assurance)

c4-judge commented 8 months ago

Trumpero marked the issue as grade-a

c4-judge commented 8 months ago

Trumpero marked the issue as grade-c