003.md

Jolly Mauve Parakeet

Medium

Project will undercharge users when using pyth oracle

Summary

perennial charging singleUpdateFeeInWei for every report, while pyth will charge that amount or more.

Root Cause

Whenever keeper commit price it will eventually go to this part of code where fee is static for every report.

    function _parsePrices(
        bytes32[] memory underlyingIds,
        bytes calldata data
    ) internal override returns (PriceRecord[] memory prices) {
        prices = new PriceRecord[](underlyingIds.length);
        bytes[] memory datas = new bytes[](1);
        datas[0] = data;

        PythStructs.PriceFeed[] memory parsedPrices = pyth.parsePriceFeedUpdates{value: msg.value}(
            datas,
            underlyingIds,
            type(uint64).min,
            type(uint64).max
        );

        uint256 updateFee = IPythStaticFee(address(pyth)).singleUpdateFeeInWei(); // @audit static charge for every feed

        for (uint256 i; i < parsedPrices.length; i++) {
            (Fixed18 significand, int256 exponent) =
                (Fixed18.wrap(parsedPrices[i].price.price), parsedPrices[i].price.expo + PARSE_DECIMALS);
            Fixed18 base = Fixed18Lib.from(int256(10 ** SignedMath.abs(exponent)));
            prices[i] = PriceRecord(
                parsedPrices[i].price.publishTime,
                exponent < 0 ? significand.div(base) : significand.mul(base),
                updateFee // @audit this will eventually be charged
            );
        }
    }

contracts/pyth/PythFactory.sol#L64 But in pyth oracle there is a specific function for how much transaction been charged in when calling parsePriceFeedUpdates above, which should be passed into created PriceRecord.price

    function getUpdateFee(
        bytes[] calldata updateData
    ) public view override returns (uint feeAmount) {
        uint totalNumUpdates = 0;
        for (uint i = 0; i < updateData.length; i++) {
            if (
                updateData[i].length > 4 &&
                UnsafeCalldataBytesLib.toUint32(updateData[i], 0) ==
                ACCUMULATOR_MAGIC
            ) {
                (
                    uint offset,
                    UpdateType updateType
                ) = extractUpdateTypeFromAccumulatorHeader(updateData[i]);
                if (updateType != UpdateType.WormholeMerkle) {
                    revert PythErrors.InvalidUpdateData();
                }
                totalNumUpdates += parseWormholeMerkleHeaderNumUpdates(
                    updateData[i],
                    offset
                );
            } else {
                totalNumUpdates += 1;
            }
        }
        return getTotalFee(totalNumUpdates);
    }

0xdd24f84d36bf92c65f92307595335bdfab5bbd21#code#F2#L107

Internal pre-conditions

The protocol will use feeds for which this will be true according to function above updateData[i].length > 4 && UnsafeCalldataBytesLib.toUint32(updateData[i], 0) == ACCUMULATOR_MAGIC

External pre-conditions

none

Attack Path

No response

Impact

Protocol will undercharge users sometimes and pay more fees by itself

PoC

No response

Mitigation

    function _parsePrices(
        bytes32[] memory underlyingIds,
        bytes calldata data
    ) internal override returns (PriceRecord[] memory prices) {
        prices = new PriceRecord[](underlyingIds.length);
        bytes[] memory datas = new bytes[](1);
        datas[0] = data;

        PythStructs.PriceFeed[] memory parsedPrices = pyth.parsePriceFeedUpdates{value: msg.value}(
            datas,
            underlyingIds,
            type(uint64).min,
            type(uint64).max
        );

-        uint256 updateFee = IPythStaticFee(address(pyth)).singleUpdateFeeInWei();
+        uint256 updateFee = IPythStaticFee(address(pyth)).getUpdateFee(datas);

        for (uint256 i; i < parsedPrices.length; i++) {
            (Fixed18 significand, int256 exponent) =
                (Fixed18.wrap(parsedPrices[i].price.price), parsedPrices[i].price.expo + PARSE_DECIMALS);
            Fixed18 base = Fixed18Lib.from(int256(10 ** SignedMath.abs(exponent)));
            prices[i] = PriceRecord(
                parsedPrices[i].price.publishTime,
                exponent < 0 ? significand.div(base) : significand.mul(base),
                updateFee
            );
        }
    }