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

slot0 is used in two function which could easily be manipulated #30

Closed
c4-bot-3 opened this issue Apr 7, 2024 · 6 comments
Closed

slot0 is used in two function which could easily be manipulated #30

c4-bot-3 opened this issue Apr 7, 2024 · 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 downgraded by judge Judge downgraded the risk level of this issue duplicate-537 partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%) 🤖_30_group AI based duplicate group recommendation

Comments

@c4-bot-3
Copy link
Contributor

c4-bot-3 commented Apr 7, 2024

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L333-L344
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticFactory.sol#L328-L411

Vulnerability details

Impact

The returned parameter currentSqrtPriceX96 from slot0 can be easily manipulated with a flashloan, resulting in a loss of funds.

Proof of Concept

The first function that calls slot0 and uses the returned price is assertPriceWithinBounds in PanopticPool.sol:

    function assertPriceWithinBounds(uint160 sqrtLowerBound, uint160 sqrtUpperBound) external view {
        (uint160 currentSqrtPriceX96,,,,,,) = s_univ3pool.slot0();

        if (currentSqrtPriceX96 <= sqrtLowerBound || currentSqrtPriceX96 >= sqrtUpperBound) {
            revert Errors.PriceBoundFail();
        }
    }

One of the comments for this function states that it can be used in a multicall, as a slippage check, for a force exercise or liquidation. If a malicious user were to front-run the call to this function with a flashloan, they could bump up the price so that their options aren't exercised or their account isn't liquidated.

The second function that utilizes slot0 is _mintFullRange in PanopticFactory.sol:

    function _mintFullRange(IUniswapV3Pool v3Pool, address token0, address token1, uint24 fee)
        internal
        returns (uint256, uint256)
    {
        (uint160 currentSqrtPriceX96,,,,,,) = v3Pool.slot0();

        uint128 fullRangeLiquidity;
        unchecked {
            // Since we know one of the tokens is WETH, we simply add 0.1 ETH + worth in tokens
            if (token0 == WETH) {
                fullRangeLiquidity =
                    uint128(Math.mulDiv96RoundingUp(FULL_RANGE_LIQUIDITY_AMOUNT_WETH, currentSqrtPriceX96));
            } else if (token1 == WETH) {
                fullRangeLiquidity = uint128(
                    Math.mulDivRoundingUp(FULL_RANGE_LIQUIDITY_AMOUNT_WETH, Constants.FP96, currentSqrtPriceX96)
                .....

A malicious user could manipulate the price so that the newly created SFPM is deployed with more liquidity than intended.

Tools Used

Manual Review

Recommended Mitigation Steps

I suggest refactoring the functions, in order for them to use Uniswap's TWAP, as it's the safer choice.

Assessed type

Oracle

@c4-bot-3 c4-bot-3 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Apr 7, 2024
c4-bot-3 added a commit that referenced this issue Apr 7, 2024
@c4-bot-13 c4-bot-13 added the 🤖_30_group AI based duplicate group recommendation label Apr 22, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as duplicate of #537

@Picodes
Copy link

Picodes commented Apr 23, 2024

For assertPriceWithinBounds as it's a query function and there is no real scenario described here I consider it to be of low severity

@c4-judge
Copy link
Contributor

c4-judge commented May 1, 2024

Picodes changed the severity to 2 (Med Risk)

@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 May 1, 2024
@c4-judge
Copy link
Contributor

c4-judge commented May 1, 2024

Picodes marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%) and removed satisfactory satisfies C4 submission criteria; eligible for awards labels May 1, 2024
@c4-judge
Copy link
Contributor

c4-judge commented May 1, 2024

Picodes marked the issue as partial-25

@Picodes
Copy link

Picodes commented May 1, 2024

Giving partial credit for the lack of PoC.

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-537 partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%) 🤖_30_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

4 participants