veRWA - Brenzee's results

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L118-L122 https://github.com/code-423n4/2023-08-verwa/blob/main/src/GaugeController.sol#L211-L278

Vulnerability details

Impact

When a new gauge is added with GaugeController:add_gauge function, users' votes do not change the Gauge's relative weight when GaugeController:vote_for_gauge_weights is called and it will always be 0.

Furthermore - this affects LendingLedger:claim function, where it would calculate users' claimable rewards for specific Gauge as 0.

Proof of Concept

Let's take GaugeController.t:testVoteDifferentTime test as an example. In this test Gauges are added to GaugeController and user1 calls vote_for_gauge_weights.

        vm.startPrank(gov);
        gc.add_gauge(gauge1);
        gc.add_gauge(gauge2);
        vm.stopPrank();

        ...

        vm.startPrank(user1);
        gc.vote_for_gauge_weights(gauge1, weights[0]);
        gc.vote_for_gauge_weights(gauge2, weights[1]);
        assertEq(gc.vote_user_power(user1), 10000);
        vm.stopPrank();

After that, there is a loop where Gauge relative weights are asserted. If we add console.logs to _rel_weight_1 and _rel_weight_2, we will see that in the loop it logs out "0" 10 times for _rel_weight_1 and _rel_weight_2.

      uint256 _rel_weigth_1 = gc.gauge_relative_weight(gauge1, block.timestamp);
      uint256 _rel_weigth_2 = gc.gauge_relative_weight(gauge2, block.timestamp);
      console.log("rel_weigth_1: ", _rel_weigth_1);
      console.log("rel_weigth_2: ", _rel_weigth_2);

Run

  forge test --mc GaugeControllerTest --match-test testVoteDifferentTime -vv

Logs

  rel_weigth_1: 0
  rel_weigth_2: 0

As a result - even though user1 has voted for both of the Gauges, the relative weight does not get affected at all for both of them.

Reason why it happens

This happens because when Gauge is added to GaugeController with add_gauge function, time_weight[_gauge_addr] is not set.

    function add_gauge(address _gauge) external onlyGovernance {
        require(!isValidGauge[_gauge], "Gauge already exists");
        isValidGauge[_gauge] = true;
        // @audit time_weight[_gauge_addr] is not set
        emit NewGauge(_gauge);
    }

time_weight[_gauge_addr] value is crucial for vote_for_gauge_weights that uses _get_weight function - if time_weight[_gauge_addr] is 0, _get_weight_ function will always return 0 and not update points_weight.

This can be proven if we set time_weight for the Gauge in add_gauge function and run the test again:

  function add_gauge(address _gauge) external onlyGovernance {
    require(!isValidGauge[_gauge], "Gauge already exists");
    isValidGauge[_gauge] = true;
    time_weight[_gauge] = ((block.timestamp + WEEK) / WEEK) * WEEK;
    emit NewGauge(_gauge);
  }

Run

  forge test --mc GaugeControllerTest --match-test testVoteDifferentTime -vv

Logs

  rel_weigth_1: 800000000000031536
  rel_weigth_2: 199999999999968463

As a result - Now relative weights are correctly calculated and returned with 1e18 accuracy.

Tools Used

Manual Review

Recommended Mitigation Steps

Set time_weight for the Gauge in add_gauge function. Example of the code below:

  function add_gauge(address _gauge) external onlyGovernance {
    require(!isValidGauge[_gauge], "Gauge already exists");
    isValidGauge[_gauge] = true;
    time_weight[_gauge] = ((block.timestamp + WEEK) / WEEK) * WEEK;
    emit NewGauge(_gauge);
  }

Assessed type

Error