Prevent markets getting stuck when prices don't move

[code423n4]

Handle

gpersoon

Vulnerability details

Impact

Suppose there is a synthetic token where the price stays constant, for example:

• synthetic DAI (with a payment token of DAI the price will not move)
• binary option token (for example tracking the USA elections; after the election results there will be no more price movements)

In that case assetPriceHasChanged will never be true (again) and marketUpdateIndex[marketIndex] will never increase.
This means the _executeOutstandingNextPrice* functions will never be executed, which means the market effectively will be stuck.

Proof of Concept

//https://github.com/code-423n4/2021-08-floatcapital/blob/main/contracts/contracts/LongShort.sol#L669
function _updateSystemStateInternal(uint32 marketIndex) internal virtual requireMarketExists(marketIndex) {
...
int256 newAssetPrice = IOracleManager(oracleManagers[marketIndex]).updatePrice();
int256 oldAssetPrice = int256(assetPrice[marketIndex]);
bool assetPriceHasChanged = oldAssetPrice != newAssetPrice;

if (assetPriceHasChanged || msg.sender == staker) {
  ....
  if (!assetPriceHasChanged) {   
    return;
  }
  ....
 marketUpdateIndex[marketIndex] += 1;  // never reaches this point if the price doesn't change

// https://github.com/code-423n4/2021-08-floatcapital/blob/main/contracts/contracts/LongShort.sol#L1035
function _executeOutstandingNextPriceSettlements(address user, uint32 marketIndex) internal virtual {
uint256 userCurrentUpdateIndex = userNextPrice_currentUpdateIndex[marketIndex][user];
if (userCurrentUpdateIndex != 0 && userCurrentUpdateIndex <= marketUpdateIndex[marketIndex]) { // needs marketUpdateIndex[marketIndex] to be increased
_executeOutstandingNextPriceMints(marketIndex, user, true);
_executeOutstandingNextPriceMints(marketIndex, user, false);
_executeOutstandingNextPriceRedeems(marketIndex, user, true);
_executeOutstandingNextPriceRedeems(marketIndex, user, false);
_executeOutstandingNextPriceTokenShifts(marketIndex, user, true);
_executeOutstandingNextPriceTokenShifts(marketIndex, user, false);

Tools Used

Recommended Mitigation Steps

Enhance _updateSystemStateInternal so that after a certain period of time without price movements (for example 1 day),
the entire function is executed (including the marketUpdateIndex[marketIndex] += 1;)

[JasoonS]

Good point, we thought the time since last update check wasn't necessary.

I'll chat with the team about what they think the risk is. But I don't think it is 3 given that we don't plan to launch any assets that don't have regular change (so market would be stuck for a limited time - even if it is long).

In a lot of ways our 'nextPriceExecution` model is designed for this case. Some more traditional markets close for the weekend and over night. Our mechanism means that users will be able to buy and trade these assets at any time and get the asset as soon as there is an update.

[JasoonS]

The more I think about this the more I think it is a safety feature. It is way more likely that if the oracle keeps returning the same value that something is broken (which means we can catch the issue before it negatively impacts the system by unfairly managing user funds or similar). If it really is stuck on the same value legitimately it can replace the OracleManager that is being used to help with that.

[moose-code]

Since there is no current plan for a binary market, and the system would definitely need other accommodations to allow a binary market, since next price execution etc, this doesn't make sense as an issue in this case. The system is built for markets where continuous price updates will occur.

Agree with Jason, if not price update is occurring, there is likely an issue with the oracle, and our system is not failing even in light of this issue. It is effectively paused until a new price update is given. As Jason mentions we can use the oracle manager to fix this

[0xean]

Due to the ability to update the oracle, funds would not be lost, but it would be an availability risk (even if temporary) for the system. Based on that I am downgrading to a 2.