Governance can change gauge weight of not valid gauge address

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/GaugeController.sol#L188-L199

Vulnerability details

Impact

The Gauge weight can be forced changed by the governance using change_gauge_weight. However unlike vote_for_gauge_weights, which checks whether the gauge address is valid using isValidGauge, change_gauge_weight is not checking this condition. Possible impact:

A malicious governance can pretend deleting a gauge which is unfavourable for the users, by removing it using remove_gauge, however it can still force change its weight leading to unfair distribution.

The malicious actor can set a very high weight to a gauge they favor (or control), which may result in that gauge receiving a disproportionate amount of rewards or influence in the system.

Proof of Concept

Add this code to:

gc.change_gauge_weight(user1, 100); assertEq(gc.get_gauge_weight(user1), 100);

https://github.com/code-423n4/2023-08-verwa/blob/a693b4db05b9e202816346a6f9cada94f28a2698/src/test/GaugeController.t.sol#L52-L63

    function testRemoveGauge() public {
    vm.startPrank(gov);

    gc.add_gauge(user1);
    assertTrue(gc.isValidGauge(user1));

    gc.remove_gauge(user1);
    assertTrue(!gc.isValidGauge(user1));
    assertTrue(gc.get_gauge_weight(user1) == 0);

    gc.change_gauge_weight(user1, 100);
    assertEq(gc.get_gauge_weight(user1), 100);

    vm.stopPrank();
}

You can observe that despite being deleted, user1 has gained again weight from 0 to 100. When the governance deleted user1, a deceiving event was created which states that user1 is removed and its weight is 0 however, one step later its weight was increased to 100. This can lead to market manipulation.

Tools Used

Foundry

Recommended Mitigation Steps

Check is it valid gauge before updating weight require(isValidGauge[_gauge], "Invalid gauge address");

Assessed type

Governance

[c4-pre-sort]

141345 marked the issue as duplicate of #36

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)