_karanel - No way to update `maxOracleUpdateDurationFeed0` and `maxOracleUpdateDurationFeed1` outside of constructor, and could lead to DOS of the Valantis module due to falsely getting stale price

[sherlock-admin4]

_karanel

high

No way to update maxOracleUpdateDurationFeed0 and maxOracleUpdateDurationFeed1 outside of constructor, and could lead to DOS of the Valantis module due to falsely getting stale price

Summary

If feedToken0 and feedToken1 were not set in the constructor later proposed by manager using HOT::proposeFeeds() and set by liquidity provider using HOT::setFeeds, and the actual maxOracleUpdateDuration (or heartbeat, in case of chainlink price feeds) for any of the token is more than what was set in the constructor (could even be 0 if not supplied same as price feeds), it could lead to denial-of-service and even locking of funds.

Vulnerability Detail

Inside the HOTOracle::constructor if price feed of tokens (i.e. feedToken0 and feedToken1) are not set then we have an internal function HOTOracle::_setFeeds which is called by the public functionHOT::setFeeds which can be only used by the liquidity provider. HOT::setFeeds can only be called after the manager proposes price feeds to be set using HOT::proposeFeeds.

If while deployment of HOT contract price feeds of the tokens were not set, some value for _maxOracleUpdateDurationFeed0 and _maxOracleUpdateDurationFeed1 would still be set (could even be 0).

After the price feeds have been set, while retrieving price feed data for these tokens using HOTOracle::_getOraclePriceUSD, the contract might assume that the oracle is returning stale data and revert the transaction with error HOTOracle___getOraclePriceUSD_stalePrice.

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOTOracle.sol#L70-L90 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOTOracle.sol#L125-L149

File: valantis-hot/HOTOracle.sol
70:     constructor(
71:         address _token0Pool,
72:        address _token1Pool,
73:         address _feedToken0,
74:         address _feedToken1,
75:         uint32 _maxOracleUpdateDurationFeed0,
76:         uint32 _maxOracleUpdateDurationFeed1
77:     ) {
78:         _token0 = _token0Pool;
79:         _token1 = _token1Pool;
80:
81:         _token0Base = 10 ** IERC20Metadata(_token0Pool).decimals();
82:         _token1Base = 10 ** IERC20Metadata(_token1Pool).decimals();
83:
84:         maxOracleUpdateDurationFeed0 = _maxOracleUpdateDurationFeed0;
85:         maxOracleUpdateDurationFeed1 = _maxOracleUpdateDurationFeed1;
86:
87:         // Feeds can be 0 during deployment, but once feeds are set, they cannot be changed.
88:         feedToken0 = AggregatorV3Interface(_feedToken0);
89:         feedToken1 = AggregatorV3Interface(_feedToken1);
90:     }
...
...
125:    function _setFeeds(address _feedToken0, address _feedToken1) internal {
126:        if (address(feedToken0) != address(0) || address(feedToken1) != address(0)) {
127:            revert HOTOracle___setFeeds_feedsAlreadySet();
128:        }
129:
130:        if (_feedToken0 == address(0) || _feedToken1 == address(0)) {
131:            revert HOTOracle___setFeeds_newFeedsCannotBeZero();
132:        }
133:
134:        feedToken0 = AggregatorV3Interface(_feedToken0);
135:        feedToken1 = AggregatorV3Interface(_feedToken1);
136:    }
137:
138:    function _getOraclePriceUSD(
139:        AggregatorV3Interface feed,
140:        uint32 maxOracleUpdateDuration
141:    ) internal view returns (uint256 oraclePriceUSD) {
142:        (, int256 oraclePriceUSDInt, , uint256 updatedAt, ) = feed.latestRoundData();
143:
144:        if (block.timestamp - updatedAt > maxOracleUpdateDuration) {
145:            revert HOTOracle___getOraclePriceUSD_stalePrice();
146:        }
147:
148:        oraclePriceUSD = oraclePriceUSDInt.toUint256();
149:    }

Impact

1. This breaks core functionality of the valantis-hot module and leads to denial-of-service.
2. This could cause locking of funds (even permanently) by breaking HOT::setPriceBounds, HOT::depositLiquidity and HOT::getLiquidityQuote functions as they underneath call HOTOracle::getSqrtOraclePriceX96 function which internally calls HOTOracle::_getOraclePriceUSD function.

Code Snippet

https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOTOracle.sol#L70-L90 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOTOracle.sol#L125-L149 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L217-L295 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L419-L428 https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L514-L518

Tool used

Manual Review

Recommendation

Allow the manager to enter the maxOracleUpdateDuration (or heartbeat) when proposing price feeds, inside the HOT::proposeFeeds function.

File: valantis-hot/HOTOracle.sol

-   uint32 public immutable override maxOracleUpdateDurationFeed0;
-   uint32 public immutable override maxOracleUpdateDurationFeed1;
+   uint32 public override maxOracleUpdateDurationFeed0;
+   uint32 public override maxOracleUpdateDurationFeed1;
...
...
-   function _setFeeds(address _feedToken0, address _feedToken1) internal {
+   function _setFeeds(address _feedToken0, address _feedToken1, uint32 _maxOracleUpdateDurationFeed0, uint32 _maxOracleUpdateDurationFeed1) internal {
        if (address(feedToken0) != address(0) || address(feedToken1) != address(0)) {
            revert HOTOracle___setFeeds_feedsAlreadySet();
        }

        if (_feedToken0 == address(0) || _feedToken1 == address(0)) {
            revert HOTOracle___setFeeds_newFeedsCannotBeZero();
        }

        feedToken0 = AggregatorV3Interface(_feedToken0);
        feedToken1 = AggregatorV3Interface(_feedToken1);
+       maxOracleUpdateDurationFeed0 = _maxOracleUpdateDurationFeed0;
+       maxOracleUpdateDurationFeed1 = _maxOracleUpdateDurationFeed1;
    }

File: valantis-hot/HOT.sol

+   uint32 public proposedMaxOracleUpdateDurationFeed0;
+   uint32 public proposedMaxOracleUpdateDurationFeed0;
...
...
-   function proposeFeeds(address _feedToken0, address _feedToken1) external onlyManager {
+   function proposeFeeds(address _feedToken0, address _feedToken1, uint32 _maxOracleUpdateDurationFeed0, uint32 _maxOracleUpdateDurationFeed1) external onlyManager {
        if (proposedFeedToken0 != address(0) || proposedFeedToken1 != address(0)) {
            revert HOT__proposedFeeds_proposedFeedsAlreadySet();
        }

        proposedFeedToken0 = _feedToken0;
        proposedFeedToken1 = _feedToken1;
+       proposedMaxOracleUpdateDurationFeed0 = _maxOracleUpdateDurationFeed0;
+       proposedMaxOracleUpdateDurationFeed1 = _maxOracleUpdateDurationFeed1;

        emit OracleFeedsProposed(_feedToken0, _feedToken1);
    }
...
...
    function setFeeds() external onlyLiquidityProvider {
-       _setFeeds(proposedFeedToken0, proposedFeedToken1);
+       _setFeeds(proposedFeedToken0, proposedFeedToken1, proposedMaxOracleUpdateDurationFeed0, proposedMaxOracleUpdateDurationFeed1);

        emit OracleFeedsSet();
    }