Float Capital contest - cmichel's results

Handle

cmichel

Vulnerability details

The LongShort._batchConfirmOutstandingPendingActions function uses the batched_amountSyntheticToken_toShiftAwayFrom_marketSide[marketIndex][false] field to determine how much short tokens to shift to long tokens.

However, this field is not cleared, instead, the long options (true) are cleared again:

// Handle shift tokens from SHORT to LONG
amountForCurrentAction_workingVariable = batched_amountSyntheticToken_toShiftAwayFrom_marketSide[marketIndex][
  false
];
if (amountForCurrentAction_workingVariable > 0) {
  int256 paymentTokenValueChangeForShiftToLong = int256(
    _getAmountPaymentToken(amountForCurrentAction_workingVariable, syntheticTokenPrice_inPaymentTokens_short)
  );

  short_changeInMarketValue_inPaymentToken -= paymentTokenValueChangeForShiftToLong;
  long_changeInMarketValue_inPaymentToken += paymentTokenValueChangeForShiftToLong;

  changeInSupply_syntheticToken_short -= int256(amountForCurrentAction_workingVariable);
  changeInSupply_syntheticToken_long += int256(
    _getEquivalentAmountSyntheticTokensOnTargetSide(
      amountForCurrentAction_workingVariable,
      syntheticTokenPrice_inPaymentTokens_short,
      syntheticTokenPrice_inPaymentTokens_long
    )
  );

  // @audit should be [false]
  batched_amountSyntheticToken_toShiftAwayFrom_marketSide[marketIndex][true] = 0;
}

Impact

An attacker can shift short to long tokens once, and can then repeatedly print long tokens and empty short tokens on every _batchConfirmOutstandingPendingActions call as this field is not cleared. This completely breaks the market as the short and long token total supply is wrong as well as the "changeInMarketValue" variables wrongly change the short/long value of the market.

User funds will be lost as the short side keeps losing in value and the long side gains value through the mints. (An attacker can also use this information to make a profit.)

Recommended Mitigation Steps

Reset the correct field by assigning batched_amountSyntheticToken_toShiftAwayFrom_marketSide[marketIndex][false] = 0;.

Handle

cmichel

Vulnerability details

The LongShort.getUsersConfirmedButNotSettledSynthBalance function estimates the outstanding synthetic tokens using the latest synthetic token price (currentMarketUpdateIndex), instead of the price at the next price update after the user deposited (userNextPrice_currentUpdateIndex[marketIndex][user]).

// see getUsersConfirmedButNotSettledSynthBalance
uint256 amountPaymentTokenDeposited = userNextPrice_paymentToken_depositAmount[marketIndex][isLong][user];
// @audit uses current price
uint256 syntheticTokenPrice = syntheticToken_priceSnapshot[marketIndex][isLong][currentMarketUpdateIndex];
confirmedButNotSettledBalance = _getAmountSyntheticToken(amountPaymentTokenDeposited, syntheticTokenPrice);

// see actual amount minted at _executeOutstandingNextPriceMints
uint256 currentPaymentTokenDepositAmount = userNextPrice_paymentToken_depositAmount[marketIndex][isLong][user];
uint256 amountSyntheticTokensToTransferToUser = _getAmountSyntheticToken(
  currentPaymentTokenDepositAmount,
  syntheticToken_priceSnapshot[marketIndex][isLong]
  // @audit uses deposit price
  [userNextPrice_currentUpdateIndex[marketIndex][user]]
);

The actual mint amount in _executeOutstandingNextPriceMints however uses the price of the next price update after the user deposit.

Impact

Any contracts or frontends relying on getUsersConfirmedButNotSettledSynthBalance will receive a wrong token balance if the account has not "claimed" yet. Note that this is also the function that SyntheticToken.balanceOf uses, the standard ERC20 function, which is used a lot. This can lead to losses if this balance is relied on in any way.

Recommendation

Use the correct synth price in the getUsersConfirmedButNotSettledSynthBalance view function.

Handle

cmichel

Vulnerability details

Minting/redeeming tokens only happens in the next "epoch" where an epoch is started only if the underlying prices change.

Impact

Stable underlyings that don't change the price often or have a low precision can lead to issues.

Recommended Mitigation Steps

Consider starting a new epoch either at each price change or after X minutes have passed to force the protocol to process outstanding actions.