code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Description

NFTFloorOracle.sol is used to query prices of NFTs. Admin has excessive privileges in this contract, which allows them to inflate the value of NFTs and take massive undercollateralized loans, or, deflate their value which would trigger liquidation of user's holdings.

The addFeeder function immediately adds a new feeder which will update the price

function addFeeders(address[] calldata _feeders)
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
{
    _addFeeders(_feeders);
}

If there are above or equal to MIN_ORACLES_NUM, the price is considered valid.

if (validNum < MIN_ORACLES_NUM) {
    return (false, assetPriceMap[_asset].twap);
}
_quickSort(validPriceList, 0, int256(validNum - 1));
return (true, validPriceList[validNum / 2]);

So, compromised owner can simple add three feeders which will call setPrice() with rogue data.

Another large centralization risk is when setting the first price on a given asset:

//first time just use the feeding value
if (assetPriceMap[_asset].twap == 0) {
    return (true, _twap);
}

Owner can abuse it easily by deleting the asset and adding it again, which will enter this "initialization" flow and accepting any given twap without combining it with other oracles.

/// @notice Allows owner to add assets.
/// @param _assets assets to add
function addAssets(address[] calldata _assets)
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
{
    _addAssets(_assets);
}
/// @notice Allows owner to remove asset.
/// @param _asset asset to remove
function removeAsset(address _asset)
    external
    onlyRole(DEFAULT_ADMIN_ROLE)
    onlyWhenAssetExisted(_asset)
{
    _removeAsset(_asset);
}

Impact

Compromised or malicious owner can fully control NFT prices, making them able to take uncollateralized loans or freely liquidate users

Tools Used

Manual audit

Recommended Mitigation Steps

Add a time delay when adding and deleting assets. Wait a certain time before new feeder's prices are accepted, to give time for users to react and flee with their assets.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #54