hats-finance / Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777

IBO, Vesting & Bond mecanism repo prepared for Hat finance audit competition
0 stars 0 forks source link

There is no sequencer check in _getPriceAggregator which will lead to stale price on L2 #22

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @ahmaddecoded Submission hash (on-chain): 0xaaddde06ade66726b5b92ed5d57ec880df9135b8312a932bb9a5612276791f62 Severity: medium

Description: Description\ Sequencer check is necessary to be compatible with all evm chains else stale price will get in as recommended by chainlink docs Attack Scenario\ If stale price gets in all the calculation and pricing will be impacted. Attachments

  1. Proof of Concept (PoC) File

    
    function getOracleAndAggregatorPrices(
        IERC20Metadata erc20Address
    ) external view returns (uint256, uint256, bool, bool, bool, bool) {
        return _getOracleAndAggregatorPrices(erc20Address);
    }
    
    /**
     *  @notice double security, compare the price computed through pool with Aggregator oracle
     */
    function getAndVerifyCvgPrice() external view returns (uint256) {
        return _getAndVerifyOracleAndAggregatorPrices(cvg);
    }
    
    /**
     *  @notice double security, compare the price computed through pool with Chainlink oracle
     *  @param erc20Address address of the token we want to fetch the price
     */
    function _getOracleAndAggregatorPrices(
        IERC20Metadata erc20Address
    ) internal view returns (uint256, uint256, bool, bool, bool, bool) {
        /// @dev Fetch price through CvgOracle
        IOracleStruct.OracleParams memory oracleParams = oracleParametersPerERC20[erc20Address];
        (uint256 poolOraclePrice, bool isEthVerified) = oracleParams.isStable
            ? (10 ** 18, true)
            : _getPriceOracle(erc20Address);
        (uint256 aggregatorOraclePrice, uint256 lastUpdateDate) = _getPriceAggregator(oracleParams.aggregatorOracle);
        uint256 delta = (oracleParams.deltaAggregatorCvgOracle * poolOraclePrice) / 10_000;
    
        return (
            poolOraclePrice,
            aggregatorOraclePrice,
            poolOraclePrice + delta > aggregatorOraclePrice,
            poolOraclePrice - delta < aggregatorOraclePrice,
            isEthVerified,
            lastUpdateDate + oracleParams.maxLastUpdateAggregator > block.timestamp
        );
    }

above functions dont check for l2 sequencer



Check similar issue at;

https://solodit.xyz/issues/m-4-no-check-for-active-arbitrum-sequencer-in-wsteth-oracle-sherlock-sentiment-sentiment-update-git

2. **Revised Code File (Optional)**
<!-- If possible, please provide a second file containing the revised code that offers a potential fix for the vulnerability. This file should include the following information:
- Comment with a clear explanation of the proposed fix.
- The revised code with your suggested changes.
- Any additional comments or explanations that clarify how the fix addresses the vulnerability. -->
walk-on-me commented 1 year ago

Hello, Thanks a lot for your attention. First of all, Convergence will be deployed on the Ethereum Mainnet. Also, we are already checking if a price is stale or not through the isStale parameter.

We have so to consider this issue as Invalid