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(...) could turn into an infinite loop #353

Closed
code423n4 opened this issue Jun 28, 2023 · 5 comments
Closed

_decrementWeightUntilFree(...) could turn into an infinite loop #353

code423n4 opened this issue Jun 28, 2023 · 5 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 downgraded by judge Judge downgraded the risk level of this issue duplicate-735 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

_decrementWeightUntilFree(...) can be stuck in an infinite loop and this will potentially cause a DOS for all function calls that depend on it.

Proof of Concept

if the weight of the gauge in the loop is 0 this function will be stuck in an infinite loop because the loop counter i will not be incremented.
For instance, if there are 10 gauges, and the 4th gauge (i = 3) has a weight of 0, the loop will continue from i = 3 and be stuck there until the caller runs out of gas this can lead to a denial of service

        for (uint256 i = 0; i < size && (userFreeWeight + totalFreed) < weight;) {
            ...
            if (userGaugeWeight != 0) {
                ...

                unchecked {
                    i++;
                }
            }
        }

Tools Used

VS code

Recommended Mitigation Steps

Bring the loop increment counter outside the if block

        for (uint256 i = 0; i < size && (userFreeWeight + totalFreed) < weight;) {
            ...
            if (userGaugeWeight != 0) {
                ...


            }
            unchecked {
               i++;
            }
        }

Assessed type

Loop

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jun 28, 2023
code423n4 added a commit that referenced this issue Jun 28, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as duplicate of #260

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

trust1995 marked the issue as satisfactory

@c4-judge c4-judge added partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed satisfactory satisfies C4 submission criteria; eligible for awards labels Jul 10, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as partial-50

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jul 11, 2023
@c4-judge
Copy link
Contributor

trust1995 changed the severity to 2 (Med Risk)

@c4-judge
Copy link
Contributor

trust1995 marked the issue as duplicate of #717

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 downgraded by judge Judge downgraded the risk level of this issue duplicate-735 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

2 participants