L01 - fillOffchainOrder is vulnerable to replay attack as signature is missing chainId parameter

Vulnerability Details

The readme states that Zaros will be deployed on 2 chains: Arbitrum and Monad.
The issue here is that chainId isn't included into the signature, making it possible for a malicious actor to replay it on the other chain.

While the keepers are trusted, the probability of a malicious actor finding a way to make this signature processed by the keepers, by exploiting an offchain path, still exists.

It is then recommended to set this probability to zero by not allowing it to be replayed on another chain.

Impact

Malicious actor can re-use a same signature for an order on both chain when it has been signed for only one chain.

Recommendations

Add a chainId field to the struct hash to ensure it cannot be reused on the other chain.

L02 - As minAnswer and maxAnswer are deprecated by Chainlink, those values must now configured by protocol itself

Summary

Protocol is using depracated values that are no longer relevant and should manage those values by itself if planning to use them.

Vulnerability Details

https://docs.chain.link/data-feeds#check-the-latest-answer-against-reasonable-limits

The data feed aggregator includes both minAnswer and maxAnswer values. On most data feeds, these values are no longer used and they do not stop your application from reading the most recent answer. For monitoring purposes, you must decide what limits are acceptable for your application.

This means that, depending on the price feed, that defensive mechanism will be useless, as aggregator.minAnswer() will return 1, and aggregator.maxAnswer() will return type(uint192).max (e.g rETH/ETH price feed returned values)

Impact

No protection against suspicious price changes.

Tools Used

Manual review

Recommendations

Add an internal management system for minAnswer and maxAnswer if planning to use them to detect suspicious price changes.

L03 - Protocol implementation contract has no receive, fallback or dedicated payable function, but need ETH to pay fees to chainlinkVerifier

Summary

When an order is filled, Chainlink Data Streams are used to get bid and ask price.
To verify the validity of the report, the protocol has chosen to pay chainlinkVerifier in chain native asset, but has no function to receive those assets, like receive, fallback or dedicated payable function.

Vulnerability Details

Data Streams report can be paid in different assets, which can be LINK, native blockchain gas tokens or their ERC20-wrapped version.
As we can see bellow, Zaros pay the chainlinkVerifier using the native blockchain gas tokens, which means he needs to have the funds to do so:

The issue is that the contract has no dedicated way to receive those funds.
To do so, the owner will have to call TradingAccountBranch::createTradingAccountAndMulticall function which is payable, but it require the caller to create a tradingAccount and will use unecessary gas.

Impact

No proper way to deposit funds for Chainlink Data Streams report verifications.

Tools Used

Manual review

Recommendations

Add a dedicated function to deposit native gas tokens to the protocol.