Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Functions calling `verifyReport` to verify offchain prices from chainlink will fail

inh3l
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.

function verifyReport(
IVerifierProxy chainlinkVerifier,
FeeAsset memory fee,
bytes memory signedReport
)
internal
returns (bytes memory verifiedReportData)
{
verifiedReportData = chainlinkVerifier.verify{ value: fee.amount }(signedReport, abi.encode(fee.assetAddress)); //@note
}

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

function verify(
bytes calldata payload,
bytes calldata parameterPayload
) external payable checkAccess returns (bytes memory) {
IVerifierFeeManager feeManager = s_feeManager;
​
// Bill the verifier
if (address(feeManager) != address(0)) {
feeManager.processFee{value: msg.value}(payload, parameterPayload, msg.sender);
}
​
return _verify(payload);
}

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.

function verifyDataStreamsReport(
DataStreamsStrategy memory dataStreamsStrategy,
bytes memory signedReport
)
internal
returns (bytes memory verifiedReportData)
{
IVerifierProxy chainlinkVerifier = dataStreamsStrategy.chainlinkVerifier;
​
bytes memory reportData = ChainlinkUtil.getReportData(signedReport);
(FeeAsset memory fee) = ChainlinkUtil.getEthVericationFee(chainlinkVerifier, reportData);
verifiedReportData = ChainlinkUtil.verifyReport(chainlinkVerifier, fee, signedReport); //@note
}

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.

function verifyOffchainPrice(
Data storage self,
bytes memory priceData,
uint256 maxVerificationDelay
)
internal
returns (UD60x18 bidX18, UD60x18 askX18)
{
if (self.strategy == Strategy.DATA_STREAMS_DEFAULT) {
DataStreamsStrategy memory dataStreamsStrategy = abi.decode(self.data, (DataStreamsStrategy));
bytes memory verifiedPriceData = verifyDataStreamsReport(dataStreamsStrategy, priceData); //@note
​
requireDataStreamsReportIsValid(dataStreamsStrategy.streamId, verifiedPriceData, maxVerificationDelay);
​
PremiumReport memory premiumReport = abi.decode(verifiedPriceData, (PremiumReport));
​
(bidX18, askX18) =
(ud60x18(int256(premiumReport.bid).toUint256()), ud60x18(int256(premiumReport.ask).toUint256()));
} else {
revert Errors.InvalidSettlementStrategy();
}
}

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.

function fillMarketOrder(
uint128 tradingAccountId,
uint128 marketId,
bytes calldata priceData
)
external
onlyMarketOrderKeeper(marketId)
{
//...
// verifies provided price data following the configured settlement strategy
// returning the bid and ask prices
(ctx.bidX18, ctx.askX18) =
settlementConfiguration.verifyOffchainPrice(priceData, globalConfiguration.maxVerificationDelay);
//...
}
function fillOffchainOrders(
uint128 marketId,
OffchainOrder.Data[] calldata offchainOrders,
bytes calldata priceData
)
external
onlyOffchainOrdersKeeper(marketId)
{
//...
// verifies provided price data following the configured settlement strategy
// returning the bid and ask prices
(ctx.bidX18, ctx.askX18) =
settlementConfiguration.verifyOffchainPrice(priceData, globalConfiguration.maxVerificationDelay);
//...

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.

function verify(
bytes calldata payload,
bytes calldata parameterPayload
) external payable checkAccess returns (bytes memory) {
IVerifierFeeManager feeManager = s_feeManager;
​
// Bill the verifier
if (address(feeManager) != address(0)) {
feeManager.processFee{value: msg.value}(payload, parameterPayload, msg.sender);
}
​
return _verify(payload);
}

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.

function processFee(
bytes calldata payload,
bytes calldata parameterPayload,
address subscriber
) external payable override onlyProxy {
//...
if (fee.assetAddress == i_linkAddress) {
_handleFeesAndRewards(subscriber, feeAndReward, 1, 0);
} else {
_handleFeesAndRewards(subscriber, feeAndReward, 0, 1);
}
}

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.

function _handleFeesAndRewards(
address subscriber,
FeeAndReward[] memory feesAndRewards,
uint256 numberOfLinkFees,
uint256 numberOfNativeFees
) internal {
//...
if (msg.value != 0) {
//there must be enough to cover the fee
if (totalNativeFee > msg.value) revert InvalidDeposit();
​
//wrap the amount required to pay the fee & approve as the subscriber paid in wrapped native
IWERC20(i_nativeAddress).deposit{value: totalNativeFee}();
​
unchecked {
//msg.value is always >= to fee.amount
change = msg.value - totalNativeFee;
}
} else {
if (totalNativeFee != 0) {
//subscriber has paid in wrapped native, so transfer the native to this contract
IERC20(i_nativeAddress).safeTransferFrom(subscriber, address(this), totalNativeFee);
}
}
//...
}

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.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

No means for the PerpEngine to receive native to pay the Chainlink Verifier in case Chainlinks charges fees to the protocol

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.