c4-bot-5 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-neobase/blob/d6e6127e6763b93c23ee95cdf7622fe950d9ed30/src/GaugeController.sol#L323-L341

Vulnerability details

Impact

The function change_gauge_weight does not check if the gauge exists before assigning a weight. This is a governance controlled function, and is thus controlled by the DAO. The issue is that since there is no sanity check, the DAO can assign weights to non-existent gauges as well.

This throws off the reward calculations since the points_total and the total_weight of the gauges is also changed. This leadst o all other legitimate gauges getting less rewards than they should.

_total_weight = _total_weight + new_sum * type_weight - old_sum * type_weight;
points_total[next_time] = _total_weight;
time_total = next_time;

There should be a sanity check to prevent this, as this can affect the rewards of the legitimate gauges.

Proof of Concept

A simple test case is enough to check if this is possible.

function testChangeWeight() public {
  vm.prank(gov);
  gc.change_gauge_weight(address(1337), 1e18);
}

The above test passes without reverting and changes the total weight as well. This will affect all legitimate gauges and decrease their rewards.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check to see if the gauge exists before changing the weight.

Assessed type

Invalid Validation

c4-judge commented 7 months ago

MarioPoneder marked the issue as primary issue

MarioPoneder commented 7 months ago

Referring to README:

Publicly Known Issues

Mistakes by Governance: We assume that all calls that are performed by the governance address are performed with the correct parameters.

Still leaving as is for now, for sponsor review.