Submitted by hyh, also found by brgltd, minhquanym, Jeiwan, and gzeon
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L278-L286
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L307-L316
NFTFloorOracles _addAsset() and _addFeeder() truncate the assets and feeders arrays indices to 255, both using uint8 index field in the corresponding structures and performing uint8(assets.length - 1) truncation on the new element addition.
2^8 - 1 looks to be too tight as an all time element count limit. It can be realistically surpassed in a couple years time, especially given multi-asset and multi-feeder nature of the protocol. This way this isnt a theoretical unsafe truncation, but an accounting malfunction that is practically reachable given long enough system lifespan, without any additional requirements as asset/feeder turnaround is a going concern state of the system.
Once truncation start corrupting the indices the asset/feeder structures will become incorrectly referenced and removal of an element will start to remove another one, permanently breaking up the structures.
This will lead to inability to control these structures and then to Oracle malfunction. This can lead to collateral mispricing. Setting the severity to be medium due to prerequisites.
feederPositionMap and assetFeederMap use uint8 indices:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L32-L48
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L79-L88
On entry removal both assets array length do not decrease:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L296-L305
On the contrary, feeders array is being decreased:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L326-L338
I.e. assets array element is set to zero with delete, but not removed from the array.
This means that assets will only grow over time, and will eventually surpass 2^8 - 1 = 255. Thats realistic given that assets here are NFTs, whose variety will increase over time.
Once this happen the truncation will start to corrupt the indices:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L278-L286
This can happen with feeders too, if the count merely surpass 255 with net additions:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L307-L316
This will lead to _removeAsset() and _removeFeeder() clearing another assets/feeders as the assetFeederMap[_asset].index and feederPositionMap[_feeder].index become broken being truncated. It will permanently mess the structures.
As a simplest measure consider increasing the limit to 2^32 - 1:
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L278-L286
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L307-L316
https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/NFTFloorOracle.sol#L32-L48
Also, consider actually removing assets array element in _removeAsset() via the usual moving of the last element as its done in _removeFeeder().
LSDan (judge) increased severity to High
