Zaros Part 1

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/external/chainlink/ChainlinkUtil.sol#L103

Summary

Missing payable and approval functionalities to send verifier fees to chainlink's feemanager contract.

Vulnerability Details

In ChainlinkUtil.sol, there's the verifyReport function which is used to verify chainlink prices using chainlinkVerifier contract. The function is intended to send the fee amount in ETH to VerifierProxy.sol, we can see that the call to chainlink verifier attempts to send the fee.amount as ETH.

And as can be seen from VerifierProxy.sol, the function expects and uses msg.value as fee amount.

This is all well and good, however, none the functions calling the verifyReport function are payable, nor do they have a receive functionality with which they can receive and later forward the ETH fee to the verifier contract.

To prove this, we'll follow the function logic.
Using the search functionality, we can see that the verifyReport function is called in SettlementConfiguration.sol library in the verifyDataStreamsReport function. The function as can be seen is not marked payable, nor does the library hold a source of receiving ETH.

And in searching for verifyDataStreamsReport function, we discover it's also in use in the verifyOffchainPrice function, also in SettlementConfiguration.sol. And as can be observed, the function is not marked payable, nor does the librarys have any way of receiving ETH.

Repeating the search process, we discover that verifyOffchainPrice is used in two major functions in SettlementBranch.sol. The fillMarketOrder and the fillOffchainOrders which are called by their keeper respective keepers.

Notice that these functions are also not marked payable, neither does the contract have a way of receiving ETH due to its lack of the receive functionality.
As a result, calls to these functions, will fail if the fee.amount is > 0 as ETH is not sent.

Now, Chainlink probabaly expected this and therefore allows the subscribers to pay in WETH instead, as the native token if ETH is not sent. Following the logic chain from the verify function in VerifierProxy.sol, the function attempts to process fees in the fee manager through the processFee function.

In the processFee function, the _handleFeesAndRewards is called using parameters for i_nativeAddress not i_linkaddress since our chainlinkUtil library uses that as our fee token and fee amount source.

In _handleFeesAndRewards, we can see how the fee is handled. If msg.value is not sent, an attempt is made to transfer the i_nativeAddress from the subscriber, in this case, our msg.sender in VerifierProxy.sol, which is the contract that started the entire chain, (not the libraries) which is SettlementBranch.sol.

And by going through SettlementBranch.sol, or the entire codebase, there's no instance of the the FeeManager being approved to transfer i_nativeAddress tokens, which according to arbiscan is WETH.

Impact

In conclusion, any of the functions that require offchain price verification stand the risk of failure, due to fees not being sent as ETH, or chainlink's fee manager being approved to transfer its wrapped counter part. And as a result, such functions will fail.
The function chain to follow goes like this:

verifyReport -> verifyDataStreamsReport -> verifyOffchainPrice -> fillOffchainOrders & fillMarketOrder
fillMarketOrder is further user in MarketOrderKeeper.sol in the performUpkeep function.

Tools Used

Manual Code Review

Recommendations

Introduce a payable or receive functionality in the needed contract. Alternatively, approve the feemanager to spend the feeamount of the nativeaddress token before verification.