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

_decrementWeightUntilFree() Possible infinite loop #735

Open
code423n4 opened this issue Jul 5, 2023 · 6 comments
Open

_decrementWeightUntilFree() Possible infinite loop #735

code423n4 opened this issue Jul 5, 2023 · 6 comments
Labels
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 M-09 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/erc-20/ERC20Gauges.sol#L547-L549

Vulnerability details

Impact

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

Proof of Concept

In the loop of the _decrementWeightUntilFree() method, the position of i++ is wrong, which may lead to a infinite loop

    function _decrementWeightUntilFree(address user, uint256 weight) internal nonReentrant {
...
        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++;
                }
            }
        }

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

Tools Used

Recommended Mitigation Steps

    function _decrementWeightUntilFree(address user, uint256 weight) internal nonReentrant {
...
        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++;
-               }
            }
+           unchecked {
+             i++;
+           }            
        }

Assessed type

Context

@code423n4 code423n4 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 Jul 5, 2023
code423n4 added a commit that referenced this issue Jul 5, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as duplicate of #260

@c4-judge
Copy link
Contributor

trust1995 marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards duplicate-717 and removed duplicate-260 labels Jul 11, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as duplicate of #717

@c4-judge c4-judge reopened this Jul 25, 2023
@c4-judge c4-judge added primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report labels Jul 25, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as selected for report

@c4-sponsor
Copy link

0xLightt marked the issue as sponsor confirmed

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jul 28, 2023
@C4-Staff C4-Staff added the M-09 label Jul 31, 2023
@0xLightt
Copy link

0xLightt commented Sep 6, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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 M-09 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants