Summary

In Zaros, having a position on a market requires 2 steps :

1. create the order

2. wait for the Chainlink keepers to fulfill it

To fulfill these orders, 2 functions are used depending on the type of order :

• SettlementBranch::fillMarketOrder()

• SettlementBranch::fillOffchainOrders()

These functions are respectively protected by the onlyMarketOrderKeeper and onlyOffchainOrdersKeeper modifiers. Both of them take a marketId as a parameter and verify msg.sender is a keeper that has been assigned to the corresponding market.

Vulnerability Details

The issue is these modifier rely the internal _requireIsKeeper function which lacks to verify the keeper has been set for the given market.

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

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

The function will revert if the sender IS NOT the keeper AND the keeper IS NOT address(0).

Only if BOTH of these conditions are met, the transaction will revert, meaning if one or both of them fails, the transaction won't revert.

This means if keeper == address(0) hence, the keeper has not been set, the transaction will proceed successfully.

Zaros aims to deploy multiple markets over its life cycle so making sure a keeper has been set is mandatory for the protocol to behave as expected.

Impact

In case no keepers has been set, everyone is considered a keeper and can fulfill any order with the ability to pass an arbitrary priceData as parameter to potentially manipulate the Zaros markets to their advantage or to the disadvantage of other traders.

Tools Used

Manual review

Recommendations

Add a requirements in the _requireIsKeeper() function to make sure a keeper has been assigned to the market.