Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible infinite loop in _decrementWeightUntilFree() #698

Closed
c4-bot-4 opened this issue Dec 27, 2023 · 4 comments
Closed

Possible infinite loop in _decrementWeightUntilFree() #698

c4-bot-4 opened this issue Dec 27, 2023 · 4 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-152 edited-by-warden grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality

Comments

@c4-bot-4
Copy link
Contributor

c4-bot-4 commented Dec 27, 2023

Lines of code

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

Vulnerability details

Impact

In _decrementWeightUntilFree() there is a great possibility of an Infinite loop. This is because i++ is an increment inside if condition. This can lead to excessive gas consumption, causing the Ethereum transaction to fail due to the gas limit.

Proof of Concept

In ERC20Gauges.sol we have _decrementWeightUntilFree() which use greedy algorithm for freeing weight.
In the loop of the _decrementWeightUntilFree() method, the position of i++ is wrong, which may lead to an infinite loop:

        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; //@audit- infinity loop
                }
            }
        }

As we can see i++ is incremented only when userGaugeWeight != 0 is true.
If we don't enter the if condition, i won't increase and so we get an infinite loop.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

To avoid this potential infinite loop, move the unchecked box outside the if condition.

Assessed type

Loop

@c4-bot-4 c4-bot-4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 27, 2023
c4-bot-10 added a commit that referenced this issue Dec 27, 2023
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Jan 3, 2024
@c4-pre-sort
Copy link

0xSorryNotSorry marked the issue as duplicate of #152

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jan 28, 2024
@c4-judge
Copy link
Contributor

Trumpero changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

Trumpero marked the issue as grade-b

@Trumpero Trumpero mentioned this issue Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-152 edited-by-warden grade-b QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants