In the `ProfitManager` contract, the credit token reward will not be assigned if there are no voters for the gauge that receives rewards

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/governance/ProfitManager.sol#L292

Vulnerability details

Impact

Upon ProfitManager receiving rewards from the lending interest payment, these "rewards" are sent to various recipients, such as surplusBuffer, GuildHolders, Credit rebase rewards and others. The issue arises if rewards are assigned to guild voters, however, there are NO guild holders who are voting for that gauge, this means that the credit token reward is NOT assigned anywhere, generating a small inflation of the credit token.

In addition, there is a comment mentioning that those profits are not assigned to the gauge if there are no voters, however the credit reward is not assigned or burned, keeping a not updated credit token:

File: ProfitManager.sol
292:     function notifyPnL(
293:         address gauge,
294:         int256 amount
295:     ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
...
...
382:             // distribute to the guild
383:             if (amountForGuild != 0) {
384:                 // update the gauge profit index
385:                 // if the gauge has 0 weight, does not update the profit index, this is unnecessary
386:                 // because the profit index is used to reattribute profit to users voting for the gauge,
387:                 // and if the weigth is 0, there are no users voting for the gauge.
388:                 uint256 _gaugeWeight = uint256(
389:                     GuildToken(guild).getGaugeWeight(gauge)
390:                 );
391:                 if (_gaugeWeight != 0) {
392:                     uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge];
393:                     if (_gaugeProfitIndex == 0) {
394:                         _gaugeProfitIndex = 1e18;
395:                     }
396:                     gaugeProfitIndex[gauge] =
397:                         _gaugeProfitIndex +
398:                         (amountForGuild * 1e18) /
399:                         _gaugeWeight;
400:                 }
401:             }
402:         }
...
...
405:     }

There is a possibility that all guild voters have left the gauge (remove votes), then there is a repayment of a loan, causing the credit token reward assigned to the guild voters NOT to be distributed and to remain NOT assigned within the ProfitManager contract.

Proof of Concept

The next test shows how gauge1 has votes from Alice, however Alice withdraws her votes, then ProfitManager obtains a profit however that profit remains NOT assigned within the ProfitManager contract. Test steps:

1. Alice votes to the gauge1.
2. Alice removes his votes from the gauge1.
3. The guildSplit has the 100% rewards just for the test purposes.
4. There is a profit to the guild holders but those rewards remains unassigned in the profitManager.

// File: GuildToken.t.sol
// $ forge test --match-test "testCreditIsNotBurned" -vvv
//
    function testCreditIsNotBurned() public {
        // When there is a credit rewards for Guild Holders but nobody has a vote for that guild,
        // the credit reward will be unassigned in the `profitManager` contract
        //
        // grant roles to test contract and setup
        vm.startPrank(governor);
        core.createRole(CoreRoles.GUILD_MINTER, CoreRoles.GOVERNOR);
        core.grantRole(CoreRoles.GUILD_MINTER, address(this));
        core.createRole(CoreRoles.GAUGE_ADD, CoreRoles.GOVERNOR);
        core.grantRole(CoreRoles.GAUGE_ADD, address(this));
        core.createRole(CoreRoles.GAUGE_PARAMETERS, CoreRoles.GOVERNOR);
        core.grantRole(CoreRoles.GAUGE_PARAMETERS, address(this));
        core.createRole(CoreRoles.GAUGE_PNL_NOTIFIER, CoreRoles.GOVERNOR);
        core.grantRole(CoreRoles.GAUGE_PNL_NOTIFIER, address(this));
        vm.stopPrank();
        // setup
        token.setMaxGauges(1);
        token.addGauge(1, gauge1);
        token.mint(alice, 100e18);
        //
        // 1. Alice votes to the `gauge1`.
        vm.prank(alice);
        token.incrementGauge(gauge1, 40e18);
        assertEq(token.getUserWeight(alice), 40e18);
        assertEq(token.userUnusedWeight(alice), 60e18);
        // roll to next block.
        vm.warp(block.timestamp + 13);
        vm.roll(block.number + 1);
        //
        // 2. Alice removes his votes from the `gauge1`.
        vm.prank(alice);
        token.decrementGauge(gauge1, 40e18);
        assertEq(token.getUserWeight(alice), 0);
        assertEq(token.userUnusedWeight(alice), 100e18);
        assertEq(token.getGaugeWeight(gauge1), 0);
        //
        // 3. The guildSplit has the 100% rewards just for the test purposes.
        vm.prank(governor);
        profitManager.setProfitSharingConfig(
            0, // surplusBufferSplit
            0, // creditSplit
            1e18, // guildSplit
            0, // otherSplit
            address(0) // otherRecipient
        );
        //
        // 4. There is a profit to the guild holders but those rewards remains unassigned in the `profitManager`.
        credit.mint(address(profitManager), 200);
        profitManager.notifyPnL(gauge1, 200);
        assertEq(credit.balanceOf(address(profitManager)), 200);
        assertEq(profitManager.surplusBuffer(), 0);
    }

Tools used

Manual review

Recommended Mitigation Steps

A guild that does not have any voters who will receive profits should assign those rewards to surplusBuffer or burn those credit tokens:

    function notifyPnL(
        address gauge,
        int256 amount
    ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
...
...
            // distribute to the guild
            if (amountForGuild != 0) {
                // update the gauge profit index
                // if the gauge has 0 weight, does not update the profit index, this is unnecessary
                // because the profit index is used to reattribute profit to users voting for the gauge,
                // and if the weigth is 0, there are no users voting for the gauge.
                uint256 _gaugeWeight = uint256(
                    GuildToken(guild).getGaugeWeight(gauge)
                );
                if (_gaugeWeight != 0) {
                    uint256 _gaugeProfitIndex = gaugeProfitIndex[gauge];
                    if (_gaugeProfitIndex == 0) {
                        _gaugeProfitIndex = 1e18;
                    }
                    gaugeProfitIndex[gauge] =
                        _gaugeProfitIndex +
                        (amountForGuild * 1e18) /
                        _gaugeWeight;
                }
++              else {
++                  // assign amountForGuild to surplusBuffer or burn it
++              }
            }
        }
...
...
    }

Assessed type

Context

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1103

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-b

[c4-judge]

Trumpero marked the issue as grade-a