Boost _ Folks Finance 33675 - [Smart Contract - Low] PythNodeprocess can revert because of incorrect casting

Submitted on Fri Jul 26 2024 06:49:20 GMT-0400 (Atlantic Standard Time) by @OxAnmol for Boost | Folks Finance

Report ID: #33675

Report type: Smart Contract

Report severity: Low

Target: https://testnet.snowtrace.io/address/0xA758c321DF6Cd949A8E074B22362a4366DB1b725

Impacts:

• Contract fails to deliver promised returns, but doesn't lose value

Description

Brief/Intro

If the pyth oracle returns the exponent < -18 then the factor.toUint256() will revert due to the incorrect casting.

Vulnerability Details

 function process(bytes memory parameters) internal view returns (NodeOutput.Data memory nodeOutput) {
        (address pythAddress, bytes32 priceFeedId, bool useEma) = abi.decode(parameters, (address, bytes32, bool));

        /// @dev using unsafe methods to avoid reverting, so this accepts old data
        IPyth pyth = IPyth(pythAddress);
        PythStructs.Price memory pythData = useEma
            ? pyth.getEmaPriceUnsafe(priceFeedId)
            : pyth.getPriceUnsafe(priceFeedId);

        /// @dev adjust the price to 18 d.p., exponent is a int32 so it could be negative or positive
        int256 factor = PRECISION + pythData.expo; // 1e18 + -8 = 1e10
        uint256 price = factor > 0
            ? pythData.price.toUint256() * (10 ** factor.toUint256())
->>          : pythData.price.toUint256() / (10 ** factor.toUint256());

        return NodeOutput.Data(price, pythData.publishTime, NodeDefinition.NodeType.PYTH, 0, 0);
    }

Here the code expects the factor to be ≥0, but in case the pythData.expo is < -18 in that case the factor will be negative, and if it is negative the toUint256 of SafeCast will revert as you can see in the code.

 function toUint256(int256 value) internal pure returns (uint256) {
       //Revert if negative
        if (value < 0) {
            revert SafeCastOverflowedIntToUint(value);
        }
        return uint256(value);
    }

Here is how the original implementation from the synthetic that handles this issue. https://github.com/Synthetixio/synthetix-v3/blob/8aff01938913983b97faa5ce082c15b86db32e0d/protocol/oracle-manager/contracts/nodes/pyth/PythNode.sol#L32

Impact Details

The pyth priceFeed where the exponent is < -18 can cause the malfunctioning of the protocol.

References

https://github.com/Folks-Finance/folks-finance-xchain-contracts/blob/fb92deccd27359ea4f0cf0bc41394c86448c7abb/contracts/oracle/nodes/PythNode.sol#L36

Proof of concept

This is a test from PythNode.test.ts, here you can see that the test reverts if we override decimals from 8 to 20.

it.only("Should process price correctly with precision smaller than exponent", async function () {
      const nodeOutput = await nodeManager.process(nodeId);
      expect(nodeOutput.price).to.equal(ethers.parseUnits(price.toString(), PRECISION - decimals));
      expect(nodeOutput.timestamp).to.equal(updateTimestamp);
      expect(nodeOutput.additionalParam1).to.equal(0);
      expect(nodeOutput.additionalParam2).to.equal(0);
    });