When calling vote_for_gauge_weights in GaugeController the changes_sum are not applied to the next points_sum

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/GaugeController.sol#L263-L278
https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/GaugeController.sol#L66-L85

Vulnerability details

Impact

Since the reduced changes_sum will never be applied to the next points_sum for the sum of the gaugs, the slope of the sum will always be to flat. This will lead to less rewards distributed to the gauges since when calculating the sum of bias it will be higher than it should be resulting in the sum being higher than the total of the bias of all gauges.

Proof of Concept

The variable changes_sum is a mapping of time (t) =>"amount that should be deducted from the slope of the sum at time(t)". This deduction from the slope of the sum is necessary because at time t the locks of uses that have voted for gauges are getting unlocked and therefore their voting power is 0. This means the slope their vote added to the sum`s slope needs to be deducted to ensure the right slope for the sum. If this would not happen, the slope of the sum would be to steep resulting in the bias of the sum decreasing to fast.

When calling vote_for_gauge_weights in GaugeController, at the end of the function the changes_sum for the sum is reduced if the user already had voted for the gauge before.

if (old_slope.end > block.timestamp) {
      // Cancel old slope changes if they still didn't happen
      ...
      changes_sum[old_slope.end] -= old_slope.slope;
    }

The chages_sum values are applied to the corresponding point when calling _get_sum, deducting them from the slope of the sum. If the old_slope.end falls at the first points_sum after the time the function vote_for_gauge_weights is called, the decrease of the slope of the sum resulting from old_slope was already applied and will not be added to the gaugs slope again. This is because _get_sum was already called in the function setting the time_sum (the time the sum was updated last) to the first points_sum after the time the function vote_for_gauge_weights is called. And only points after time_sum are updated with the corresponding chages_sum. This will result in a slope for the sum that is more flat than it should be. This makes the bias of the sum bigger than it should be and therefor will lead to less rewards being distributed than the governance has approved.

Example:

The next points_sum of the gauge after the function is called will be referred to “next point of sum” = NPS.
Alice calls vote_for_gauge_weights for a gauge that she has voted before. This means that there is an old slope that will be applied. The old_slope looks like this:

old_slope = 
struct VotedSlope {
    uint256 slope = 10
    uint256 power = 500
    uint256 end = NPS
  }

In the beginning of vote_for_gauge_weights the function _get_sum is called, updating all points_sum since they were last updated ending in updating the point for NPS. When updating the point for NPS, the slope of the sum is reduced by 10 since old_slope.end is NPS and time_sum (the last time the sum points were updated) is set to NPS.

The new voting weight Alice is wanting is applied and since old_slope.end > block.timestamp the changes_sum[NPS] is reduced by 10 to account for the change. But this change will never be applied since even if anyone calls _get_sum (the only function where change_sum is used) the point at NPS will not be adjusted since time_sum was set to NPS and only points after time_sum are adjusted when calling _get_sum.

Tools Used

Manual review

Recommended Mitigation Steps

Check if old_slope.end equals the time of the next points_sum after the function call. If so explicitly adjust the slope of this point by adding the old_slope.slope to the sum slope to compensate the reduction of the slope before.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as duplicate of #72

[c4-judge]

alcueca marked the issue as not a duplicate

[alcueca]

I'm expecting that this finding will be invalid for the same reason as #72, but it is a different one. I'm going to need @OpenCoreCH to weigh in here.

[OpenCoreCH]

Yes, the same argument as in #72 applies here, as the logic for manipulating changes_sum is the same.

[c4-judge]

alcueca marked the issue as unsatisfactory:
Invalid