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

The returned value for "observe" call in twapFilter doesn't round up for negative tick deltas #248

Open
c4-bot-8 opened this issue Apr 19, 2024 · 2 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-195 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_195_group AI based duplicate group recommendation

Comments

@c4-bot-8
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L253
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1450

Vulnerability details

Impact

The twapFilter function is used in the getUniV3TWAP function, which is used to get current tick when the liquidate and forceExercise functions. This will cause that in case of a negative tick delta, the returned tick will be much bigger is desired and opens up cases of price manipiulations and arbitrage.

Proof of Concept

The twapFilter function is used to get TWAP prices over a series of time intervals. The function uses univ3pool.observe(secondsAgos) to get tickCumulatives array which is then used to calculate int24 twapMeasurement. As the univ3pool.observe() function returns negative tickCummulative delta values sometimes, these values need to be rounded down which is what is noticed upon comparison with uniswap's oracle library.

The function however account for this.

            // observe the tickCumulative at the 20 pre-defined time slots
            (int56[] memory tickCumulatives, ) = univ3pool.observe(secondsAgos);

            // compute the average tick per 30s window
            for (uint256 i = 0; i < 19; ++i) {
                twapMeasurement[i] = int24(
                    (tickCumulatives[i] - tickCumulatives[i + 1]) / int56(uint56(twapWindow / 20))
                );
            }

Tools Used

Manual code review

Recommended Mitigation Steps

Tick should be rounded down in that case:

    if ((tickCumulatives[1] - tickCumulatives[0]) < 0 && ((tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0)) tick--;

Assessed type

Uniswap

@c4-bot-8 c4-bot-8 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 Apr 19, 2024
c4-bot-3 added a commit that referenced this issue Apr 19, 2024
@c4-bot-12 c4-bot-12 added the 🤖_195_group AI based duplicate group recommendation label Apr 22, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as duplicate of #195

@c4-judge
Copy link
Contributor

c4-judge commented May 6, 2024

Picodes changed the severity to QA (Quality Assurance)

@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 May 6, 2024
@C4-Staff C4-Staff reopened this May 13, 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-195 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_195_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

4 participants