Zaros Part 1

Summary

By commitOrder then performing account transfer immediately after, a malicious trader gains control over whether his order is filled, allowing for risk-free trades and possible DOS of fillOffChainOrders.

Vulnerability Detail

During fillOffchainOrders(), the transaction reverts if the signer is not the account's owner:

After commiting an order malicious trader can manipulate order fulfilment, by performing an account transfer immediately, if he does not want the order to be filled. Then, when price or conditions moves in his favor, the trader can then transfer the account back and allow the order to be filled.

Impact

This causes several issues:

1. The malicious trader gains granular control over whether his trades are fulfilled or not, at the price he desires. Also, unlike the typical flow of commitOrder -> wait for order to fill, through this method the trader can achieve a filled order faster. These advantages may earn the malicious trader more profits at the expense of the protocol/LPs who are the counterparty providing the liquidity.

2. Denial-of-Service(DOS) of fillOffChainOrders, which aims to fill a batch of orders. Other users' orders are not filled or delayed, resulting in loss of potential profits from trades. This can be performed repeatedly and at no cost to the attacker. Keepers also waste gas in the attempts to fill the orders.

Code Snippet

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L269

Tool used

Manual Review

Recommendation

To prevent this attack vector, consider adding a time-delay for account transfers so that transferring the account right after fillOrder is not possible.

To prevent DOS, use continue instead of revert if the signer is not the account's owner. This is similar to here(https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L291) if the fillPrice is invalid.