code-423n4 / 2024-05-predy-findings

10 stars 9 forks source link

Price calculation susceptible to flashloan attacks #304

Closed thebrittfactor closed 2 months ago

thebrittfactor commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/Perp.sol#L202 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/UniHelper.sol#L13 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/Reallocation.sol#L92 https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/UniHelper.sol#L35

Vulnerability details

Impact

Contract uses uniswap slot0 price instead of TWAP price. slot0 price can be manipulated with flash loans.

Proof of Concept

The function reallocate fetches the current price from Uniswap's slot0, which can be manipulated by flash loans:

  function reallocate(
        DataType.PairStatus storage _assetStatusUnderlying,
        SqrtPerpAssetStatus storage _sqrtAssetStatus
    ) internal returns (bool, bool, int256 deltaPositionBase, int256 deltaPositionQuote) {

@--->        (uint160 currentSqrtPrice, int24 currentTick,,,,,) = IUniswapV3Pool(_sqrtAssetStatus.uniswapPool).slot0();

      //  //    //
    }

Other instance :

    function getSqrtPrice(address uniswapPoolAddress) internal view returns (uint160 sqrtPrice) {
 @-->       (sqrtPrice,,,,,,) = IUniswapV3Pool(uniswapPoolAddress).slot0();
    }

    function isInRange(Perp.SqrtPerpAssetStatus memory sqrtAssetStatus) internal view returns (bool) {
    @--->>    (, int24 currentTick,,,,,) = IUniswapV3Pool(sqrtAssetStatus.uniswapPool).slot0();

        return _isInRange(sqrtAssetStatus, currentTick);
    }

The usage of slot0 to determine deviation could potentially allow malicious actors to manipulate the deviation calculation by altering the most recent price

Tools Used

Manual Review

Recommended Mitigation Steps

Considering the potential risks associated with using slot0 to calculate deviation, implementing a Time-Weighted Average Price (TWAP) to determine the price is recommended. By providing a more accurate and harder to manipulate price point, TWAP would yield a more accurate deviation calculation. This would reduce the possibility of incorrect rebalancing and the risk of DoS attacks. Here is an example implementation of TWAP.

Assessed type

Other

thebrittfactor commented 2 months ago

Per judge request, this finding has been duplicated from it's original issue #158.

c4-judge commented 2 months ago

alex-ppg marked the issue as duplicate of #209

c4-judge commented 2 months ago

alex-ppg changed the severity to 3 (High Risk)

c4-judge commented 2 months ago

alex-ppg marked the issue as partial-50

alex-ppg commented 2 months ago

A penalty of 50% has been applied due to the submission's sub-par quality.