Gauge will be unusable if it has been inactive for too long.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L152
https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L91-#L114

Vulnerability details

Impact

Gauge will will be unusable if inactive for too long as its relative weight will always remain 0.

Proof of Concept

    function _get_weight(address _gauge_addr) private returns (uint256) {
        uint256 t = time_weight[_gauge_addr];
        if (t > 0) {
            Point memory pt = points_weight[_gauge_addr][t];
            for (uint256 i; i < 500; ++i) {
                if (t > block.timestamp) break;
                t += WEEK;
                uint256 d_bias = pt.slope * WEEK;
                if (pt.bias > d_bias) {
                    pt.bias -= d_bias;
                    uint256 d_slope = changes_weight[_gauge_addr][t];
                    pt.slope -= d_slope;
                } else {
                    pt.bias = 0;
                    pt.slope = 0;
                }
                points_weight[_gauge_addr][t] = pt;
                if (t > block.timestamp) time_weight[_gauge_addr] = t;
            }
            return pt.bias;
        } else {
            return 0;
        }
    }

With the current implementation of the code, get_weight is responsible for updating the gauge weight for every week. The problem is that the loop responsible for doing so has limited iterations (500) and only updates the value of time_weight[_gauge_addr] if the value of t > block.timestamp. This means that if it goes through all 500 iterations and t <= block.timestamp, the value of time_weight will never be updated so any calls in the future to _get_weight will just iterate over the old timestamps and values and will never set new ones.
This could be caused by 2 different situations - 1. The gauge simply has been inactive for 500 weeks and 2. The Gauge has been previously removed and is now readded 500 weeks later.
In both scenarios _get_weight will be unable to update the new values and any users trying to claim rewards via LendingLedger will get 0, despite having put their votes towards a gauge.

Foundry test

   function testInactiveGauge() public {
        vm.startPrank(gov);
        gc.add_gauge(gauge1);
        gc.change_gauge_weight(gauge1, 1e9);
        vm.stopPrank();

        vm.startPrank(user1);
        ve.createLock{value: 1 ether}(1 ether);
        gc.vote_for_gauge_weights(gauge1, 10000);
        uint weight = gc.get_gauge_weight(gauge1);
        console.log("gauge's weight after voting: ", weight);
        vm.warp(block.timestamp + 2 weeks);  // @audit going into the future as relative Weight only works properly in the past
        uint relativeWeight = gc.gauge_relative_weight_write(gauge1, block.timestamp - 1 weeks);
        console.log("Gauge's relative weight after voting: ", relativeWeight);

        vm.warp(block.timestamp + 505 weeks);
        weight = gc.get_gauge_weight(gauge1);
        console.log("gauge's weight after 505 weeks ", weight);
        relativeWeight = gc.gauge_relative_weight_write(gauge1, block.timestamp);
        console.log("Gauge's relative weight after 505 weeks", relativeWeight);
        vm.stopPrank();
        
    }

Logs:

[PASS] testInactiveGauge() (gas: 38095788)
Logs:
  gauge's weight after voting:  993424658416307200
  Gauge's relative weight after voting:  1000000000000000000
  gauge's weight after 505 weeks  985753425540505600
  Gauge's relative weight after 505 weeks 0

Tools Used

Manual review

Recommended Mitigation Steps

In get_weight even if t < block.timestamp, still set time_weight[_gauge_addr] = t;

Assessed type

Error

[c4-pre-sort]

141345 marked the issue as duplicate of #129

[c4-pre-sort]

141345 marked the issue as duplicate of #384

[c4-judge]

alcueca marked the issue as not a duplicate

[alcueca]

@OpenCoreCH, would you have a look at this one?

[OpenCoreCH]

Known issue from the Curve codebase that the function has to be called at least once every ~5 years, was stated in the contest description (to be fair it was only stated for VotingEscrow, although GaugeController has the same assumptions)

[c4-judge]

alcueca marked the issue as unsatisfactory:
Out of scope

[deadrosesxyz]

Hey, just wanted to add that as stated in the original issue, this would be the case not only if no one has called the function for ~5 years, but also if the gauge has previously been removed and has been re-added ~5+ years later. If the gauge has been removed it cannot be expected that the updating function will be called, so this is a realistic scenario where the gauge is re-added after a long time and it doesn't work properly.

[alcueca]

To remove and then add the same gauge after 5 years is a remote edge case which would nonetheless be easily found while testing such a governance action. Accepting it as QA to be added to the documentation.

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-b