Zaros Part 1

Summary

In SettlementBranch, the function fillOffchainOrders handling offchain orders contains a vulnerability where users can grief other users by signing the same order multiple times or signing invalid orders and sending it to keeper. This can cause a denial of service (DoS) by preventing valid orders from being processed.

Vulnerability Details

The code relies on a unique salt to prevent order replay. However, users can sign the same order multiple times with the same salt. When keeper calls fillOffchainOrders with same order signed by users, it will be filled once but revert second time. Due to this, the function fillOffchainOrderswill revert resulting in not processing of other users.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L256-L258

There are many scenarios where _fillOrderreverts and it's being called by fillOffchainOrders. If any one order is invalid, it will cause the whole transaction to revert.

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L343-L517

Example Scenario

1. User A creates an order and signs it with a specific salt or signs some order which is going to be reverted.

2. User A, sends the order to the keeper.

3. When keeper calls fillOffchainOrdersalong with orders of other users, the function will revert.

Impact

Causes denial of service in fillOffchainOrders.

Tools Used

Manual review

Recommendations

Instead of reverting for invalid orders, the function should return so that other orders can be processed.