Incorrect balance computed in `getUsersConfirmedButNotSettledSynthBalance()`

[code423n4]

Handle

hack3r-0m

Vulnerability details

Consider the following state:

long_synth_balace = 300;
short_synth_balace = 200;

marketUpdateIndex[1] = x;
userNextPrice_currentUpdateIndex = 0;
userNextPrice_syntheticToken_toShiftAwayFrom_marketSide[1][true] = 0;
batched_amountSyntheticToken_toShiftAwayFrom_marketSide[1][true] = 0;

User calls shiftPositionFromLongNextPrice(marketIndex=1, amountSyntheticTokensToShift=100)

This results in following state changes:

long_synth_balace = 200;
short_synth_balace = 200;
userNextPrice_syntheticToken_toShiftAwayFrom_marketSide[1][true] = 100;
batched_amountSyntheticToken_toShiftAwayFrom_marketSide[1][true] = 100;
userNextPrice_currentUpdateIndex = x+1 ;

Due to some other transactions, oracle updates twice, and now the marketUpdateIndex[1] is x+2 and also updating price snapshots.

When User calls getUsersConfirmedButNotSettledSynthBalance(user, 1)

initial condition

if (
      userNextPrice_currentUpdateIndex[marketIndex][user] != 0 &&
      userNextPrice_currentUpdateIndex[marketIndex][user] <= currentMarketUpdateIndex
    ) 

will be true;

syntheticToken_priceSnapshot[marketIndex][isLong][currentMarketUpdateIndex]
(https://github.com/hack3r-0m/2021-08-floatcapital/blob/main/contracts/contracts/LongShort.sol#L532)

this uses price of current x+2 th update while it should balance of accounting for price of x+1 th update.

[JasoonS]

Yes good spot.

This function is view only, and is only used for view only purposes. The rest of the system will always operate correctly because it rather uses _executeOutstandingNextPriceSettlements than the getUsersConfirmedButNotSettledSynthBalance. Therefore I propose this as a 1 Low Risk vulnerability.

[0xean]

I am going to align with the 2 (Med Risk) severity. Reporting the incorrect position in a UI to a user could definitely lead unexpected loss of funds in a sharp market move where a user is intending on hedging elsewhere.