Submitted by TheSchnilch
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/lib.rs#L1541-L1574
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/omnipool/src/traits.rs#L164-L190
https://github.com/code-423n4/2024-02-hydradx/blob/603187123a20e0cb8a7ea85c6a6d718429caad8d/HydraDX-node/pallets/ema-oracle/src/lib.rs#L558-L566
If an asset is removed from the omnipool, it is ensured that all data records in the omnipool are deleted and also all positions from liquidity providers. However, the data records in the Oracle are not reset. This means that if the asset is to be added again after some time and it then has a different price, the price in the Oracle is falsified.
Here is a PoC which can be inserted into the file integration-tests/src/omnipool_init.rs, which can be started with the following command: SKIP_WASM_BUILD=1 cargo test -p runtime-integration-tests poc -- --nocapture
If an asset is removed from the Omnipool, all of its data will be deleted from the Omnipool. However, the data from the asset remains in the Oracle and is not deleted there. The Oracle then continues to store the data of the removed asset in this StorageMap:
See ema-oracle/src/lib.rs#L153-L162.
The price is stored here once for the last block and once with the exponential moving average logic for a few blocks in the past. (This is the short period)
The problem now is when an asset is added again and its price has changed. As a result, the new price is calculated using the exponential moving average logic with the entries of the blocks before the asset was removed and the new entries that have been added since the asset was re-added, which leads to an incorrect price. The wrong price can also cause ensureprice in addliquidity to fail and therefore no liquidity can be added:
See omnipool/src/lib.rs#L597-L603.
Any protocol that uses this oracle as a price source would receive an incorrect price for the re-added asset for a short period of time.
Even if the price balances out again after some time (the length of the short period), an incorrect price is initially calculated after re-adding an asset for which the price has changed. I would recommend deleting the Oracle entries when removing an asset. This means that the price of the asset can be calculated correctly from the start when it is added again.
Oracle
enthusiastmartin (HydraDX) acknowledged, but disagreed with severity and commented:
Lambda (judge) commented:
