v2.1 - RFX Exchange

Every call to _validatePrices should come from a payable function as it sends a fee to the Pyth provider BUT the functions validatePrices and setPricesForAtomicAction do not have such a keyword, as oposed to setPrices, so they render unusable. The place where ETH is needed is here.

The affected functions are:

https://github.com/relative-finance/rfx-contracts/blob/0466b4a880d7d5b87ff75b639358fbe53db3b313/contracts/oracle/Oracle.sol#L179C1-L184C6

    function validatePrices(
        OracleUtils.SetPricesParams memory params,
        bool forAtomicAction
    ) external onlyController returns (OracleUtils.ValidatedPrice[] memory) {
        return _validatePrices(params, forAtomicAction);
    }

https://github.com/relative-finance/rfx-contracts/blob/0466b4a880d7d5b87ff75b639358fbe53db3b313/contracts/oracle/Oracle.sol#L119C1-L125C6

    function setPricesForAtomicAction(
        OracleUtils.SetPricesParams memory params
    ) external onlyController {
        OracleUtils.ValidatedPrice[] memory prices = _validatePrices(params, true);

        _setPrices(prices);
    }

it can be seen they do NOT have the payable keyword, so calls to these functions can NOT carry ETH. However, they both call _validatePrices, which eventually calls:

https://github.com/relative-finance/rfx-contracts/blob/0466b4a880d7d5b87ff75b639358fbe53db3b313/contracts/oracle/Oracle.sol#L294

            OracleUtils.ValidatedPrice memory validatedPrice = IOracleProviderPayable(provider).getOraclePrice{value: fee}(
                token,
                data
            );

but no ETH can be attached, so no fee can be sent to the Pyth provider, which reverts the transaction in those situation, rendering this core functionality useless.

It is recommended to add the payable keyword to those functions that deal with msg.value, so that sending them ETH does not revert and can behave correctly.

Remediation Plan

SOLVED: The Relative Finance team solved this issue by adding the payable keyword in the affected functions:

https://github.com/relative-finance/rfx-contracts/blob/53b871efdaa63437c8397aa7bae4f3cdb6364ae5/contracts/oracle/Oracle.sol#L179C1-L184C6

    function validatePrices(
        OracleUtils.SetPricesParams memory params,
        bool forAtomicAction
    ) external payable onlyController returns (OracleUtils.ValidatedPrice[] memory) {
        return _validatePrices(params, forAtomicAction);
    }

https://github.com/relative-finance/rfx-contracts/blob/53b871efdaa63437c8397aa7bae4f3cdb6364ae5/contracts/oracle/Oracle.sol#L119C1-L125C6

    function setPricesForAtomicAction(
        OracleUtils.SetPricesParams memory params
    ) external payable onlyController {
        OracleUtils.ValidatedPrice[] memory prices = _validatePrices(params, true);

        _setPrices(prices);
    }

Inside the OracleModule contract, withOraclePricesForAtomicAction modifier, there is a call to setPricesForAtomicAction inside the Oracle contract, which calls _validatePrices and so it sends a given fee to the Pyth provider for its services. However, no ETH is sent (without taking into account the previous issue around the payable keyword), so the Pyth endpoint will revert the transaction as no fee was provided.

Pretty visual:

https://github.com/relative-finance/rfx-contracts/blob/53b871efdaa63437c8397aa7bae4f3cdb6364ae5/contracts/oracle/OracleModule.sol#L35C1-L41C6

    modifier withOraclePricesForAtomicAction(
        OracleUtils.SetPricesParams memory params
    ) {
        oracle.setPricesForAtomicAction(params);
        _;
        oracle.clearAllPrices();
    }

As seen before, the call to setPricesForAtomicAction needs some ETH to be used as fees, but no ETH is sent, so any function with this modifier, namely WithdrawalHandler::executeAtomicWithdrawal renders useless.

It is recommended to send the required fee attached to the call to oracle.setPricesForAtomicAction, so that the ETH is sent through the contract to the Pyth endpoint.

Remediation Plan

SOLVED: The Relative Finance team solved this issue by sending the required fee alongside the call to the oracle:

https://github.com/relative-finance/rfx-contracts/pull/47/files#diff-ec9855fdb537cbb2d7605899ff9818f4988dbd02b59ccd34f148e721d5c0bc3c

    modifier withOraclePricesForAtomicAction(
        OracleUtils.SetPricesParams memory params
    ) {
        oracle.setPricesForAtomicAction{value: msg.value}(params);
        _;
        oracle.clearAllPrices();
    }

The PythDataStreamProvider contract does not perform input validation on the price, conf, and expo values returned from the called price feed, which can lead to the contract accepting invalid or untrusted prices. Those values should be checked as clearly stated in the official documentation.

The contract should revert the transaction here if one of the following conditions is triggered:

• price <= 0

• expo < -18

• conf > 0 && (price / int64(conf) < MIN_CONF_RATIO for a given MIN_CONF_RATIO

Remediation Plan

SOLVED: The Relative Finance team solved this issue by checking the recommended conditions, and reverting the transaction if the values were not the expected ones.