sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

ctf_sec - Compromised admin can maxOracleFreshnessInSeconds to 0 to block oracle and block trading in the TradingModule.sol #114

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

medium

Compromised admin can maxOracleFreshnessInSeconds to 0 to block oracle and block trading in the TradingModule.sol

Summary

Compromised admin can maxOracleFreshnessInSeconds to 0 to block trading.

Vulnerability Detail

The Parameter maxOracleFreshnessInSecond in the TradingModule.sol is used to ensure that the oracle price data we get is up-to-date.

   (/* */, int256 basePrice, /* */, uint256 bpUpdatedAt, /* */) = baseOracle.oracle.latestRoundData();
        require(block.timestamp - bpUpdatedAt <= maxOracleFreshnessInSeconds);
        require(basePrice > 0); /// @dev: Chainlink Rate Error

        (/* */, int256 quotePrice, /* */, uint256 qpUpdatedAt, /* */) = quoteOracle.oracle.latestRoundData();
        require(block.timestamp - qpUpdatedAt <= maxOracleFreshnessInSeconds);
        require(quotePrice > 0); /// @dev: Chainlink Rate Error

However, this parameter is set by notional admin

 function setMaxOracleFreshness(uint32 newMaxOracleFreshnessInSeconds) external onlyNotionalOwner {
        emit MaxOracleFreshnessUpdated(maxOracleFreshnessInSeconds, newMaxOracleFreshnessInSeconds);
        maxOracleFreshnessInSeconds = newMaxOracleFreshnessInSeconds;
 }

because the parameter maxOracleFreshnessInSeconds is lack of restriction and lack of lower bound and upper bound,

the admin can set the maxOracleFreshnessInSeconds to any value. A compromised admin can set the maxOracleFreshnessInSeconds to 0,

then

require(block.timestamp - qpUpdatedAt <= maxOracleFreshnessInSeconds);

would revert.

Impact

Compromised admin can maxOracleFreshnessInSeconds to 0 to block oracle and block trading in the TradingModule.sol

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingModule.sol#L220-L231

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingModule.sol#L59-L62

Tool used

Manual Review

Recommendation

We recommend the project add upper bound and lower bound restriction to the valued maxOracleFreshnessInSeconds