code-423n4 / 2024-09-fenix-finance-findings

0 stars 0 forks source link

Gauge creation should be allowed only after receiving rewards from the minter per epoch #19

Closed c4-bot-6 closed 2 months ago

c4-bot-6 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-09-fenix-finance/blob/main/contracts/core/VotingEscrowUpgradeableV2.sol#L660-L675

Vulnerability details

Impact

Rewards cannot be distributed to some gauges.

Proof of Concept

When a gauge is created, the gaugesState.index is set to the current index at L669.

File: contracts\core\VoterUpgradeableV2.sol
662:         gaugesState[gauge_] = GaugeState({
663:             isGauge: true,
664:             isAlive: true,
665:             internalBribe: internalBribe_,
666:             externalBribe: externalBribe_,
667:             pool: pool_,
668:             claimable: 0,
669:@>           index: index,
670:             lastDistributionTimestamp: 0
671:         });

If the gauge is created before calling notifyRewardAmount, the gaugesState.index is set to the old index. As a result, the new gauge can receive more rewards than the actual rewards, while other gauges may not receive any rewards.

Let's consider the following scenario:

  1. There is poolA and gaugeA, with index initially set to 0.
  2. Users vote 100 for poolA in the first week: weightsPerEpoch = 100, totalWeightsPerEpoch = 100.
  3. The governor creates gaugeB for poolB at the start of the second week: gaugesState[gaugeB] = 0.
  4. After that, users call the distribute function for poolA, and the minter transfers 100 FNX. They do not call the function for poolB because they did not vote for it.
    • index = 100 * 1e18 / 100 = 1e18
    • gaugesState[gaugeA].index = 1e18
    • gaugesState[gaugeB].index = 0
  5. Users vote 50 for both poolA and poolB individually in the second week:
    • weightsPerEpoch[poolA] = 50
    • weightsPerEpoch[poolB] = 50
    • totalWeightsPerEpoch = 100
  6. In the third week, users call the distribute function for both pools, and the minter transfers another 100 FNX.
    • index = index + 100 * 1e18 / 100 = 2e18
    • rewards amount for gaugeA: 50 * (2e18 - 1e18) / 1e18 = 50
    • rewards amount for gaugeB: 50 * (2e18 - 0) / 1e18 = 100

Even though the VoterUpgradeableV2 contract receives 100 FNX from the minter, it attempts to transfer 150 FNX to the gauges. As a result, the rewards distribution to the gauges is reverted.

Tools Used

Manual Review

Recommended Mitigation Steps

Gauge creation should only be permitted after receiving rewards from the minter for each epoch.

Assessed type

Other

c4-judge commented 2 months ago

alcueca changed the severity to 3 (High Risk)

c4-judge commented 2 months ago

alcueca marked the issue as primary issue

c4-judge commented 2 months ago

alcueca marked the issue as duplicate of #16

c4-judge commented 1 month ago

alcueca changed the severity to 2 (Med Risk)

c4-judge commented 1 month ago

alcueca marked the issue as satisfactory