IllIllI - Limit orders can be used to get a free look into the future #130
Description
IllIllI
high
Limit orders can be used to get a free look into the future
Summary
Users can continually update their orders to get a free look into prices in future blocks
Vulnerability Detail
Order execution relies on signed archived prices from off-chain oracles, where each price is stored along with the block range it applies to, and limit orders are only allowed to execute with oracle prices where the block is greater than the block in which the order was last updated. Since prices are required to be future prices, there is a time gap between when the last signed price was archived, and the new price for the next block is stored in the archive, and the order keeper is able to fetch it and submit an execution for it in the next block.
The example given by the sponsor in discord was:
the oracle process:
1. the oracle node checks the latest price from reference exchanges and stores it with the oracle node's timestamp, e.g. time: 1000
2. the oracle node checks the latest block of the blockchain, e.g. block 100, it stores this with the oracle node's timestamp as well
3. the oracle node signs minOracleBlockNumber: 100, maxOracleBlockNumber: 100, timestamp: 1000, price: <price>
4. the next time the loop runs is at time 1001, if the latest block of the blockchain is block 105, e.g. if 5 blocks were produced in that one second, then the oracle would sign
minOracleBlockNumber: 101, maxOracleBlockNumber: 105, timestamp: 1001, price: <price>
https://discord.com/channels/812037309376495636/1073619363518758972/1083555347672862820
Impact
If a user has a pending exit order that was submitted a block N, and the user sees that the price at block N+1 will be more favorable, they can update their exit order, changing the amount by +/- 1 wei, and have the order execution delayed until the next block, at which point they can decided again whether the price and or impact is favorable, and whether to exit. In the sponsor's example, if the order was submitted at block 101, they have until block 105 to decide whether to update their order, since the order execution keeper won't be able to do the execution until block 106. There is a gas cost for doing such updates, but if the position is large enough, or the price is gapping enough, it is worth while to do this, especially if someone comes up with an automated service that does this on your behalf.
The more favorable price for the attacker is at the expense of the other side of the trade, and is a loss of capital for them.
Code Snippet
Limit orders are executed by keepers and the keepers are required to provide signed prices after the last order update:
// File: gmx-synthetics/contracts/order/DecreaseOrderUtils.sol : DecreaseOrderUtils.validateOracleBlockNumbers() #1
139 if (
140 orderType == Order.OrderType.LimitDecrease ||
141 orderType == Order.OrderType.StopLossDecrease
142 ) {
143 uint256 latestUpdatedAtBlock = orderUpdatedAtBlock > positionIncreasedAtBlock ? orderUpdatedAtBlock : positionIncreasedAtBlock;
144 @> if (!minOracleBlockNumbers.areGreaterThan(latestUpdatedAtBlock)) {
145 OracleUtils.revertOracleBlockNumbersAreSmallerThanRequired(minOracleBlockNumbers, latestUpdatedAtBlock);
146 }
147 return;
148: }// File: gmx-synthetics/contracts/order/DecreaseOrderUtils.sol : DecreaseOrderUtils.validateOracleBlockNumbers() #2
139 if (
140 orderType == Order.OrderType.LimitDecrease ||
141 orderType == Order.OrderType.StopLossDecrease
142 ) {
143 uint256 latestUpdatedAtBlock = orderUpdatedAtBlock > positionIncreasedAtBlock ? orderUpdatedAtBlock : positionIncreasedAtBlock;
144 @> if (!minOracleBlockNumbers.areGreaterThan(latestUpdatedAtBlock)) {
145 OracleUtils.revertOracleBlockNumbersAreSmallerThanRequired(minOracleBlockNumbers, latestUpdatedAtBlock);
146 }
147 return;
148: }// File: gmx-synthetics/contracts/order/SwapOrderUtils.sol : SwapOrderUtils.validateOracleBlockNumbers() #3
66 if (orderType == Order.OrderType.LimitSwap) {
67 @> if (!minOracleBlockNumbers.areGreaterThan(orderUpdatedAtBlock)) {
68 OracleUtils.revertOracleBlockNumbersAreSmallerThanRequired(minOracleBlockNumbers, orderUpdatedAtBlock);
69 }
70 return;
71: }The ExchangeRouter.updateOrder() function directly updates the storage details of the order (including touching the orderUpdatedAtBlock), without any execution keeper delay, and has no extra fees associated with it.
Tool used
Manual Review
Recommendation
Require a delay between when the order was last increased/submitted, and when an update is allowed, similar to REQUEST_EXPIRATION_BLOCK_AGE for the cancellation of market orders