code-423n4 / 2024-04-panoptic-findings

9 stars 4 forks source link

Core function can be called externally leading to DOS #567

Closed c4-bot-9 closed 7 months ago

c4-bot-9 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L241-L268 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1450-L1452 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1179 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1017

Vulnerability details

Summary

Function can be called by anyone which could lead to DOS or returning a stale TWAP by providing zero as the twap window

Detailed Description

If an attacker front-runs and calls the twapFilter function with 0 as the twapWindow, it would result in a division by zero error inside the function. This would cause the function to revert, as Solidity throws an exception when attempting to divide by zero.

Now, let's consider the subsequent behavior when the liquidate function attempts to get the TWAP tick using int24 twapTick = getUniV3TWAP();. Since the twapFilter function reverted due to a division by zero error, it did not produce a valid TWAP tick value. In this case, the value of twapTick would be undefined or could potentially revert as well if it doesn't handle the case where the TWAP calculation fails gracefully.

Assuming the value of twapTick remains undefined, the comparison Math.abs(currentTick - twapTick) > MAX_TWAP_DELTA_LIQUIDATION in the liquidate function would likely cause the function to revert. This is because attempting to perform arithmetic operations with an undefined value would result in unpredictable behavior and could lead to a revert due to an invalid comparison or arithmetic operation.

Therefore, in summary:

Impact:

May become impossible to liquidate assets or forceExercise a position

Proof of Code

    function twapFilter(IUniswapV3Pool univ3pool, uint32 twapWindow) external view returns (int24) {
        uint32[] memory secondsAgos = new uint32[](20);

        int256[] memory twapMeasurement = new int256[](19);

        unchecked {
            // construct the time stots

            for (uint256 i = 0; i < 20; ++i) {
                secondsAgos[i] = uint32(((i + 1) * twapWindow) / 20);
            }

            // 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))
                );
            }

            // sort the tick measurements
            int256[] memory sortedTicks = Math.sort(twapMeasurement);

            // Get the median value
            return int24(sortedTicks[10]);
        }
    }

Tool used:

Manual Review

Recommended Mitigation:

Change the contract logic to change the external function to internal

Assessed type

DoS

Picodes commented 7 months ago

This is a library ?

c4-judge commented 7 months ago

Picodes marked the issue as unsatisfactory: Invalid