_decrementWeightUntilFree() Possible infinite loop in ERC20Gauges.sol

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/ERC20Gauges.sol#L532-L534

Vulnerability details

Impact

The position of i++ is wrong, which may lead to an infinite loop.

The incrementer is present inside the if statement. Thus if a userGaugeWeight has 0 weight, the code will get stuck in an infinite loop.

This issue with the FEI contracts was also reported during the MAIA DAO audit here.

_decrementWeightUntilFree is used in transfer, transferFrom and _burn functions, which can lead to a lot of issues.

Proof of Concept

 function _decrementWeightUntilFree(address user, uint256 weight) internal {
        //...
        for (
            uint256 i = 0;
            i < size && (userFreeWeight + userFreed) < weight;

        ) {
            address gauge = gaugeList[i];
            uint256 userGaugeWeight = getUserGaugeWeight[user][gauge];
            if (userGaugeWeight != 0) {
                userFreed += userGaugeWeight;
                _decrementGaugeWeight(user, gauge, userGaugeWeight);

                // If the gauge is live (not deprecated), include its weight in the total to remove
                if (!_deprecatedGauges.contains(gauge)) {
                    totalTypeWeight[gaugeType[gauge]] -= userGaugeWeight;
                    totalFreed += userGaugeWeight;
                }

                unchecked {
@>                  ++i;
                }
            }
        }

In the above code, when userGaugeWeight == 0, i is not incremented, resulting in an infinite loop.
The current protocol does not restrict getUserGaugeWeight[user][gauge] == 0.

Tools Used

Manual Review

Recommended Mitigation Steps

Move the i++ statement outside the if statement.

Assessed type

Loop

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #152

[c4-judge]

Trumpero changed the severity to QA (Quality Assurance)

[c4-judge]

Trumpero marked the issue as grade-b

[c4-judge]

Trumpero marked the issue as grade-c