Users can frontrun ProfitManager.notifyPnL to avoid losses or claim rewards.

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

ProfitManager.notifyPnL is used to distribute rewards or notify gauge loss.
When amount is less than 0, it will notify gauge losse and users will lose the guild tokens incremented to the gauge. And in SurplusGuildMinter, users also lose pledged credit tokens.

    function notifyPnL(
        address gauge,
        int256 amount
    ) external onlyCoreRole(CoreRoles.GAUGE_PNL_NOTIFIER) {
        uint256 _surplusBuffer = surplusBuffer;
        uint256 _termSurplusBuffer = termSurplusBuffer[gauge];
        address _credit = credit;

        // handling loss
        if (amount < 0) {
            uint256 loss = uint256(-amount);

            // save gauge loss
            GuildToken(guild).notifyGaugeLoss(gauge);

When amount is greater than 0, the reward is distributed.

        else if (amount > 0) {
            ...
            if (amountForCredit != 0) {
                CreditToken(_credit).distribute(amountForCredit);
            }

            // 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;
                }
            }

And since the protocol does not implement a locking mechanism, users can add or remove tokens at any time.
This allows users to frontrun ProfitManager.notifyPnL to avoid losing or claiming rewards.

For example, since the auction is linear and bad debts are only generated after the midpoint which makes pnl negative, users can avoid losses by unstaking tokens from SurplusGuildMinter in advance of the midpoint.

In addition, when interest is incurred due to user repays or bids, the user can frontrun the transaction to increment to the gauge and decrement as soon as getting the reward.

Proof of Concept

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

Tools Used

None

Recommended Mitigation Steps

It is recommended that instead of allowing the user to make immediate exit, the user would be required to make a exit request and would be allowed to exit after a period, and then the exit would only be allowed within a limited time frame.

Assessed type

Context

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #877

[Trumpero]

Partial duplicate of #994

[c4-judge]

Trumpero marked the issue as not a duplicate

[c4-judge]

Trumpero marked the issue as duplicate of #994

[c4-judge]

Trumpero marked the issue as unsatisfactory:
Invalid

[c4-judge]

Trumpero marked the issue as satisfactory

[Slavchew]

Hey, @Trumpero,

This issue doesn’t show the real impact mentioned in the #994

The only impact which is common for both reports is this line:

“In addition, when interest is incurred due to user repays or bids, the user can frontrun the transaction to increment to the gauge and decrement as soon as getting the reward.”

The whole report clearly emphasizes the notifyPnL frontrun part which means it is a duplicate of #877

Thanks for your time.

[Trumpero]

Agree that this issue is similar to both #994 and #877, but #994 is a higher severity issue. Due to the lack of quality regarding it, I will decrease the credit of this issue.

[c4-judge]

Trumpero marked the issue as partial-50