Position can be immediately liquidated after opening

Summary

Position can be immediately liquidated after opening by timing the submission of the Pyth price, leading to a loss of assets for the victim (Long traders)

Vulnerability Detail

At $T2$, Keeper A executes open Bob's position with the existing on-chain Pyth price of \$1000. Note that the on-chain price on the Pyth contract is already outdated compared to the latest Pyth available off-chain. However, per the design, when the order is executed at $T2$, any price data with a timestamp between $T1$ and $T2$ is considered valid and can be used within the executeOpen function to execute an open order.

In addition, when executing the open position, the protocol does not require the keeper to update the price as long as the current price is still valid (within the range of $T1$ and $T2$. Thus, this provides the keepers executing the orders an option to update the price or not.

The following shows that an acceptable oracle price that can be used when executing the order is between the current time (block.timestamp) and the order executable time (_executableAtTime).

File: LeverageModule.sol
446:     /// @notice Returns the maximum age of the oracle price to be used.
447:     /// @param _executableAtTime The time at which the order is executable.
448:     /// @return _maxAge The maximum age of the oracle price to be used.
449:     function _getMaxAge(uint64 _executableAtTime) internal view returns (uint32 _maxAge) {
450:         return (block.timestamp - _executableAtTime).toUint32();
451:     }

In this scenario, Keeper A chooses not to update the Pyth price when executing Bob's open order at T2. Thus, the order is executed with the current (outdated - In Blue) price of \$1000 per ETH. Assume that Bob's position order is as follows:

Margin = 2 ETH
Position Size = 50 ETH

Note: For simplicity's sake, assume that the market is perfectly hedged; thus, there is no accrued funding fee.

When executing Bob's open order, Keeper A could skip or avoid the updating of the Pyth price by passing in an empty updateData array. Inspecting the source code of Pyth on-chain contract, the oracleOracle.updatePriceFeeds function will not perform any update (for-loop is not executed as length = 0).

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/DelayedOrder.sol#L386

File: DelayedOrder.sol
378:     function executeOrder(
379:         address account,
380:         bytes[] calldata priceUpdateData
381:     )
382:         external
383:         payable
384:         nonReentrant
385:         whenNotPaused
386:         updatePythPrice(vault, msg.sender, priceUpdateData)
387:         orderInvariantChecks(vault)
388:     {

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L69

File: OracleModule.sol
64:     function updatePythPrice(address sender, bytes[] calldata priceUpdateData) external payable nonReentrant {
65:         // Get fee amount to pay to Pyth
66:         uint256 fee = offchainOracle.oracleContract.getUpdateFee(priceUpdateData);
67: 
68:         // Update the price data (and pay the fee)
69:         offchainOracle.oracleContract.updatePriceFeeds{value: fee}(priceUpdateData);
70: 
71:         if (msg.value - fee > 0) {
72:             // Need to refund caller. Try to return unused value, or revert if failed
73:             (bool success, ) = sender.call{value: msg.value - fee}("");
74:             if (success == false) revert FlatcoinErrors.RefundFailed();
75:         }
76:     }

For this specific position, if the price of ETH drops from \$1000 to \$960, the PnL of the position will be as follows, and Bob's position will become liquidatable.

priceShift = $960 - $1000 = -$40
PnL = (position size * priceShift) / current price
PnL = 50 ETH * -$40 / $960
PnL = -2.08 ETH

Since there is no minimum waiting or delay for executing liquidation on-chain, Keeper A could back-run its previous executeOpen TX with a liquidation TX that liquidates Bob's position. The liquidation TX will update the Pyth price to the new off-chain price (\$960 - In Red). When the liquidate function attempts to compute Bob's position remaining margin with the updated price, it will return (2 ETH - 2.08 ETH = -0.08), which indicates the position is underwater and liquidatable.

By doing so, Keeper A will not only gain the keeper fee from the first executeOpen TX, but also the keeper fee + liquidation fee from the second liquidation TX. Bob incurs a loss here as his position is closed immediately after opening, and he lost his margin, which is paid to the liquidator (as a liquidation fee) and the LPs (remaining margin).

Sidenote: The oracle's maxDiffPercent check will not guard against this attack effectively. For instance, in the above example, if the Chainlink price is \$975 and the maxDiffPercent is 5%, the Pyth price of \$960 or \$1000 still falls within the acceptable range. If the maxDiffPercent is reduced to a smaller margin, it will potentially lead to a more serious issue where all the transactions get reverted when fetching the price, breaking the entire protocol.

Impact

Loss of assets for the victim (Long traders) as shown in the scenario above.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/DelayedOrder.sol#L386

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L69

Tool used

Manual Review

Recommendation

Following are some of the measures that can be applied to mitigate the issue:

Do not allow liquidation to happen on the same block
Ensure that the order is always executed with the latest Pyth price. No skipping or avoiding the price update.
Two-stage liquidation process with delay

Duplicate of #194

sherlock-audit / 2023-12-flatmoney-judging

xiaoming90 - Position can be immediately liquidated after opening #183