Ethereum Credit Guild - zhaojie's results

Lines of code

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

Vulnerability details

Impact

A malicious user can stake for the guild token before ProfitManager#notifyPnL and then unstake after earning the proceeds, The malicious user did not contribute anything to the protocol, but reaped the benefits.

Proof of Concept

In ProfitManager#notifyPnL function, distribute to the guild,

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

claimGaugeRewards calculates gauge rewards by measuring the difference between _gaugeProfitIndex and the user's GaugeWeight.

  function claimGaugeRewards(
        address user,
        address gauge
    ) public returns (uint256 creditEarned) {
        .....
        uint256 deltaIndex = _gaugeProfitIndex - _userGaugeProfitIndex;
        if (deltaIndex != 0) {
            creditEarned = (_userGaugeWeight * deltaIndex) / 1e18;
            userGaugeProfitIndex[user][gauge] = _gaugeProfitIndex;
        }
        if (creditEarned != 0) {
            emit ClaimRewards(block.timestamp, user, gauge, creditEarned);
            CreditToken(credit).transfer(user, creditEarned);
        }
    }

The problem, is that a malicious user can execute the stake function before ProfitManager#notifyPnL to increase their GaugeWeight and reap the benefits. After receiving the revenue, the malicious user then unstake, so that he has nothing to lose, as long as the rewards are greater than the gas fee, the malicious user can profit.

    //@audit Execute before ProfitManager#notifyPnL
   function stake(address term, uint256 amount) external whenNotPaused {
        .....
        //@audit incrementGauge
        GuildToken(guild).incrementGauge(term, guildAmount);
        .....
    }

    //@audit Execute after ProfitManager#notifyPnL
    function unstake(address term, uint256 amount) external {
        // apply pending rewards
        //@audit got rewards
        (, UserStake memory userStake, bool slashed) = getRewards(
            msg.sender,
            term
        );
       .....
    }

Tools Used

vscode, manual

Recommended Mitigation Steps

Take time into account when calculating guild rewards.

Assessed type

Other