code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

Reactivated gauges can’t queue up rewards #883

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/rewards/rewards/FlywheelGaugeRewards.sol#L189-L193

Vulnerability details

Impact

Reactivated gauges can’t queue up rewards

Proof of Concept

Active gauges as set by authorised users get their rewards queued up in the FlywheelGaugeRewards._queueRewards() function. As part of it, their associated struct QueuedRewards updates its storedCycle value to the cycle in which they get queued up:

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/rewards/rewards/FlywheelGaugeRewards.sol#L189-L193

gaugeQueuedRewards[gauge] = QueuedRewards({
    priorCycleRewards: queuedRewards.priorCycleRewards + completedRewards,
    cycleRewards: uint112(nextRewards),
    storedCycle: currentCycle
});

However, these gauges may be deactivated and they will now be ignored in either FlywheelGaugeRewards.queueRewardsForCycle() or FlywheelGaugeRewards.queueRewardsForCyclePaginated() because both use gaugeToken.gauges() to get the set of gauges for which to queue up rewards for the cycle, and that only gives active gauges. Therefore, any updates FlywheelGaugeRewards makes to its state will not be done to deactivated gauges' QueuedRewards structs. In particular, the gaugeCycle contract state variable will keep advancing throughout its cycles, while QueuedRewards.storedCycle will retain its previously set value, which is the cycle where it was queued and not 0.

Once reactivated later with at least 1 full cycle being done without it, it will produce issues. It will now be returned by gaugeToken.gauges() to be processed in either FlywheelGaugeRewards.queueRewardsForCycle()or FlywheelGaugeRewards.queueRewardsForCyclePaginated(), but, once the reactivated gauge is passed to _queueRewards(), it will fail an assert:

https://github.com/code-423n4/2023-05-maia/blob/cfed0dfa3bebdac0993b1b42239b4944eb0b196c/src/rewards/rewards/FlywheelGaugeRewards.sol#L183

assert(queuedRewards.storedCycle == 0 || queuedRewards.storedCycle >= lastCycle);

This is because it already has a set value from the cycle it was processed in previously (i.e. storedCycle>0), and, since that cycle is at least 1 full cycle behind the state contract, it will also not pass the second condition queuedRewards.storedCycle >= lastCycle.

The result is that this gauge is locked out of queuing up for rewards because queuedRewards.storedCycle is only synchronised with the contract’s cycle later in _queueRewards() which will now always fail for this gauge.

Tools Used

Code inspection

Recommended Mitigation Steps

Account for the reactivated gauges that previously went through the rewards queue process, such as introducing a separate flow for newly activated gauges. However, any changes such as removing the above mentioned assert() should be carefully validated for other downstream logic that may use the QueuedRewards.storedCycle value. Therefore, it is recommended to review the state transitions as opposed to only passing this specific check.

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid