ERC20Gauges.sol: infinite loop if one of the user's gauges has a zero weight

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/erc-20/ERC20Gauges.sol#L548

Vulnerability details

Impact

When a user transfers or burns his tokens, the algorithm is freeing weights from the gauges that belong to him. This is done inside a for loop that iterates over the list of user's gauges _userGauges[user].values(), if the gauge has a non-zero weight we remove it, unfortunately loop counter is placed inside the if block if (userGaugeWeight != 0) making it impossible to proceed if gauge weight is zero, therefore transfer/burn will fail out of gas.

I'm putting this bug in medium because the only way for this to happen is when the user increments his gauge with a zero value.

Proof of Concept

Test case in ERC20GaugesTest.t.sol

    function testIncrementGauges() public {
        token.mint(address(this), 100e18);
        token.setMaxDelegates(1);
        token.delegate(address(this));

        token.setMaxGauges(2);
        token.addGauge(gauge1);
        token.addGauge(gauge2);

        token.incrementGauge(gauge1, 0);
        token.incrementGauge(gauge2, 100e18);

        address to = hevm.addr(100);
        token.transfer(to, 100e18);
    }

Tools Used

Forge

Recommended Mitigation Steps

Put i++ outside of the if block

        for (uint256 i = 0; i < size && (userFreeWeight + totalFreed) < weight;) {
            address gauge = gaugeList[i];
            uint112 userGaugeWeight = getUserGaugeWeight[user][gauge];
            if (userGaugeWeight != 0) {
                // If the gauge is live (not deprecated), include its weight in the total to remove
                if (!_deprecatedGauges.contains(gauge)) {
                    totalFreed += userGaugeWeight;
                }
                userFreed += userGaugeWeight;
                _decrementGaugeWeight(user, gauge, userGaugeWeight, currentCycle);

            }
            unchecked {
                i++;
            }
        }

Assessed type

DoS

[c4-judge]

trust1995 marked the issue as primary issue

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 marked the issue as duplicate of #717