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

Insolvent user could avoid liquidation due to logic error #186

Open
c4-bot-1 opened this issue Apr 18, 2024 · 5 comments
Open

Insolvent user could avoid liquidation due to logic error #186

c4-bot-1 opened this issue Apr 18, 2024 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-421 edited-by-warden grade-a Q-36 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_31_group AI based duplicate group recommendation

Comments

@c4-bot-1
Copy link
Contributor

c4-bot-1 commented Apr 18, 2024

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#L1017-L1171

Vulnerability details

Impact

Available possibility for loss of funds, due to unexpected liquidation DoS

Proof of Concept

At the end of PanopticMath::twapFilter, the tick at sortedTicks[10] is returned, which isn't at the median of sortedTicks with 19 length, which leads to permanent returning of higher tick than actual.

There are 2 main scenarios which could occur due to bad indexation:

  1. When liquidator executes PanopticPool::liquidate, the function could revert unexpectedly due to (Math.abs(currentTick - twapTick) > MAX_TWAP_DELTA_LIQUIDATION) won't pass, because of currentTick - twapTick is going to be higher than it is supposed to be (if actualTwapTick > currentTick).
  2. During the execution of PanopticPool::liquidate, PanopticPool::_getSolvencyBalances is called to compute the balanceCross and thresholdCross.
function _getSolvencyBalances(
        LeftRightUnsigned tokenData0,
        LeftRightUnsigned tokenData1,
        uint160 sqrtPriceX96
    ) internal pure returns (uint256 balanceCross, uint256 thresholdCross){
        unchecked {
            // the cross-collateral balance, computed in terms of liquidity X*√P + Y/√P
            // We use mulDiv to compute Y/√P + X*√P while correctly handling overflows, round down
            balanceCross =
                Math.mulDiv(uint256(tokenData1.rightSlot()), 2 ** 96, sqrtPriceX96) +
                Math.mulDiv96(tokenData0.rightSlot(), sqrtPriceX96);
            // the amount of cross-collateral balance needed for the account to be solvent, computed in terms of liquidity
            // overstimate by rounding up
            thresholdCross =
                Math.mulDivRoundingUp(uint256(tokenData1.leftSlot()), 2 ** 96, sqrtPriceX96) +
                Math.mulDiv96RoundingUp(tokenData0.leftSlot(), sqrtPriceX96);
        }
    }

The third parameter from the input is the sqrtPrice at the twapTick. Due to the logic error the sqrtPrice is permanently going to be higher than actual, because of this the chases of (balanceCross >= thresholdCross) check to revert are permanently increased (if tokenData0.rightSlot() > tokenData0.leftSlot()).

Tools Used

Recommended Mitigation Steps

Rewrite the end of the function to get the median value from 9th index instead of 10th

function twapFilter(IUniswapV3Pool univ3pool, uint32 twapWindow) external view returns (int24) {
        ...
        ...
        ...
            // Get the median value
+++         return int24(sortedTicks[9]);
---         return int24(sortedTicks[10]);
        }
    }

Assessed type

Other

@c4-bot-1 c4-bot-1 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 18, 2024
c4-bot-6 added a commit that referenced this issue Apr 18, 2024
@c4-bot-13 c4-bot-13 added the 🤖_31_group AI based duplicate group recommendation label Apr 22, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as duplicate of #239

@c4-judge
Copy link
Contributor

Picodes marked the issue as not a duplicate

@c4-judge
Copy link
Contributor

Picodes marked the issue as duplicate of #421

@c4-judge c4-judge removed the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Apr 29, 2024
@c4-judge
Copy link
Contributor

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 labels Apr 29, 2024
@c4-judge
Copy link
Contributor

Picodes marked the issue as grade-a

@C4-Staff C4-Staff reopened this May 13, 2024
@C4-Staff C4-Staff added the Q-36 label 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-421 edited-by-warden grade-a Q-36 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_31_group AI based duplicate group recommendation
Projects
None yet
Development

No branches or pull requests

5 participants