c4-bot-8 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/tokens/ERC20Gauges.sol#L515-L536

Vulnerability details

Impact

The _decrementWeightUntilFree function contains a logic flaw that may result in an incorrect calculation of the totalWeight and totalTypeWeight values. This issue can lead to inaccurate representations of the user's free weight and the overall system's total weight, potentially affecting the proper functioning of the gauge system.

Proof of Concept

The vulnerability arises from the placement of the unchecked { ++i; } statement within the if statement block inside the for loop, causing the loop to potentially skip certain iterations and leading to an incomplete update of the totalWeight variable. This can result in an inconsistent state where the total weight is not accurately reflecting the actual weights of the gauges, impacting the reliability of the gauge system.

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;
            }

            //@audit should be placed outside of the if statement
            unchecked {
                ++i;
            }
        }
    }

    totalWeight -= totalFreed;
}

By moving the unchecked { ++i; } statement outside the if statement, the loop will increment properly, ensuring that all user gauges are properly processed and the totalWeight is updated accurately.

Tools Used

Manual review

Recommended Mitigation Steps

To address this issue, relocate the unchecked { ++i; } statement outside the if statement to ensure that the for loop increments correctly, allowing all relevant user gauges to be processed.

Assessed type

Loop

c4-pre-sort commented 10 months ago

0xSorryNotSorry marked the issue as sufficient quality report

c4-pre-sort commented 10 months ago

0xSorryNotSorry marked the issue as duplicate of #152

c4-judge commented 9 months ago

Trumpero changed the severity to QA (Quality Assurance)