code-423n4 / 2022-11-paraspace-findings

7 stars 4 forks source link

the MAX_DEVIATION_RATE restriction can reject valid NFT price oracle. #447

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L14 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L371 https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L206

Vulnerability details

Impact

the MAX_DEVIATION_RATE restriction can reject valid NFT price oracle.

Proof of Concept

the MAX_DEVIATION_RATE is hardcoded in the NFTFloorPrice.sol

//reject when price increase/decrease 1.5 times more than original value
uint128 constant MAX_DEVIATION_RATE = 150;

then we set:

_setConfig(EXPIRATION_PERIOD, MAX_DEVIATION_RATE);

the parameter is used below:

function _checkValidity(address _asset, uint256 _twap)
    internal
    view
    returns (bool)
{
    require(_twap > 0, "NFTOracle: price should be more than 0");
    PriceInformation memory assetPriceMapEntry = assetPriceMap[_asset];
    uint256 _priorTwap = assetPriceMapEntry.twap;
    uint256 _updatedAt = assetPriceMapEntry.updatedAt;
    uint256 priceDeviation;
    //first price is always valid
    if (_priorTwap == 0 || _updatedAt == 0) {
        return true;
    }
    priceDeviation = _twap > _priorTwap
        ? (_twap * 100) / _priorTwap
        : (_priorTwap * 100) / _twap;

    // config maxPriceDeviation as multiple directly(not percent) for simplicity
    if (priceDeviation >= config.maxPriceDeviation) {
        return false;
    }
    return true;
}

note the line:

    // config maxPriceDeviation as multiple directly(not percent) for simplicity
    if (priceDeviation >= config.maxPriceDeviation) {
        return false;
    }
    return true;

if the price moves below or above MAX_DEVIATION_RATE, the price update is rejected.

dataValidity = _checkValidity(_asset, _twap);
require(dataValidity, "NFTOracle: invalid price data");

but here is the issue: NFT print is very volatitle, if can increase by 200% of drop 60%, in the case when the NFT price does drop or increase, the onchain data rejects the valid price.

Consider this case:

  1. a NFT is worth 80 ETH.
  2. suddenly the NFT project rugs and the price drops to 10 ETH.
  3. The deviation is too high and there is no way to adjust MAX_DEVIATION_RATE and the valid price cannot be recorded as TWAP price on-chain.
  4. User's position cannot liquidated and user consider to borrow as if the NFT worht 80 ETH and leave the project in a insovlent position.

Tools Used

Manual Review

Recommended Mitigation Steps

Check timestamp of the updated price, make the MAX_DEVIATION_RATE adjustable.

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Overinflated severity

dmvt commented 1 year ago

This can be easily fixed by the admin already. The suggested mitigation has no real effect.