Difference in array input length for `addAsset` function can result in unexpected behaviour

Summary

addAsset function does not check for array inputs length to be equal for observations_ and feeds__ which can result in unexpected behaviour when adding assets like validating feeds from strategy.

Vulnerability Detail

Difference in array input length for addAsset function can result in unexpected behaviour

Impact

addAsset function does not have check for array length of inputs to be of same length which can result in wrong price feed resulting from empty observation or feeds.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/OlympusPrice.v2.sol#L354

Tool used

Manual Review

Recommendation

The recommendation is made to check for lengths of array inputs to avoid difference in array input length that can lead to unforeseen situation like wrong price feed.

    function addAsset(
        address asset_,
        bool storeMovingAverage_,
        bool useMovingAverage_,
        uint32 movingAverageDuration_,
        uint48 lastObservationTime_,
        uint256[] memory observations_,
        Component memory strategy_,
        Component[] memory feeds_
    ) external override permissioned {
        // Check that asset is a contract
        if (asset_.code.length == 0) revert PRICE_AssetNotContract(asset_);
+   (observations_.length == feeds_.length, “array inputs differ in length”);

        Asset storage asset = _assetData[asset_];

        // Ensure asset is not already added
        if (asset.approved) revert PRICE_AssetAlreadyApproved(asset_);

        // If not storing the moving average, validate that it's not being used by the strategy
        if (useMovingAverage_ && !storeMovingAverage_)
            revert PRICE_ParamsStoreMovingAverageRequired(asset_);

        // Strategy cannot be zero if number of feeds + useMovingAverage is greater than 1
        if (
            (feeds_.length + (useMovingAverage_ ? 1 : 0)) > 1 &&
            fromSubKeycode(strategy_.target) == bytes20(0)
        )
            revert PRICE_ParamsStrategyInsufficient(
                asset_,
                abi.encode(strategy_),
                feeds_.length,
                useMovingAverage_
            );

        // Update asset strategy data
        _updateAssetPriceStrategy(asset_, strategy_, useMovingAverage_);

        // Update asset price feed data
        _updateAssetPriceFeeds(asset_, feeds_);

        // Update asset moving average data
        _updateAssetMovingAverage(
            asset_,
            storeMovingAverage_,
            movingAverageDuration_,
            lastObservationTime_,
            observations_
        );

        // Validate configuration
        _getCurrentPrice(asset_);

        // Set asset as approved and add to array
        asset.approved = true;
        assets.push(asset_);

        // Emit event
        emit AssetAdded(asset_);
    }

sherlock-audit / 2023-11-olympus-judging

SAAJ - Difference in array input length for ``` addAsset``` function can result in unexpected behaviour #207