code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Validation of chainlink answer may use stale price #174

Closed c4-bot-2 closed 5 months ago

c4-bot-2 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L150

Vulnerability details

Details

The WiseOracleHub uses a process of validating the price from the chainlink feed by ensuring there is no significant difference between this price and twap value providing from a trusted Uniswap V3 oracle. Additionally, there are additional checks to ensure the price of chainlink provided is not stale by checking the heartbeat of the pricefeed. However, the price of chainlink used to check validation does not consider this heartbeat and only checks this outside of validation.

file: 2024-02-wise-lending/contracts/WiseOracleHub/OracleHelper.sol
  function latestResolver(
        address _tokenAddress
    )
        public
        view
        returns (uint256)
    {
        if (chainLinkIsDead(_tokenAddress) == true) {
            revert OracleIsDead();
        }
@>       return _validateAnswer(
            _tokenAddress
        );
    }

Impact

The impact is that although the chainlink heartbeat is checked when calling the latestResolver, the actual price used in validating the chainlink may be actually stale and give a false positive result where an old price is used to validate but it passes the heartbeat check because this is only checked outside of validation.

Proof of Concept

The price of chainlink used to check validation does not consider this heartbeat and only checks this outside of validation.

file: 2024-02-wise-lending/contracts/WiseOracleHub/OracleHelper.sol
 function _validateAnswer(
        address _tokenAddress
    )
        internal
        view
        returns (uint256)
    {
        UniTwapPoolInfo memory uniTwapPoolInfoStruct = uniTwapPoolInfo[
            _tokenAddress
        ];

        uint256 fetchTwapValue;

        if (uniTwapPoolInfoStruct.oracle > ZERO_ADDRESS) {
            fetchTwapValue = latestResolverTwap(
                _tokenAddress
            );
        }

@>       uint256 answer = _getChainlinkAnswer(
            _tokenAddress
        );

function _getChainlinkAnswer( address _tokenAddress ) internal view returns (uint256) { @> ( , int256 answer, , , ) = priceFeed[_tokenAddress].latestRoundData();

    return uint256(
        answer
    );
}

Tools Used

Manual review

Recommended Mitigation

  1. Check the uptime of the chainlink feed within the _validateAnswer function ensuring that only the latest answer is used to validate the price.

    function _validateAnswer(
        address _tokenAddress
    )
        internal
        view
        returns (uint256)
    {
        UniTwapPoolInfo memory uniTwapPoolInfoStruct = uniTwapPoolInfo[
            _tokenAddress
        ];
    
        uint256 fetchTwapValue;
    
        if (uniTwapPoolInfoStruct.oracle > ZERO_ADDRESS) {
            fetchTwapValue = latestResolverTwap(
                _tokenAddress
            );
        }
    +      if (chainLinkIsDead(_tokenAddress) == true) {
    +           revert OracleIsDead();
        }
        uint256 answer = _getChainlinkAnswer(
            _tokenAddress
        );
     .................................................

Assessed type

Oracle

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago

validation is done here https://github.com/code-423n4/2024-02-wise-lending/blob/1240a22a3bbffc13d5f8ae6300ef45de5edc7c19/contracts/WiseOracleHub/OracleHelper.sol#L556-L586

    function _chainLinkIsDead(
        address _tokenAddress
    )
        internal
        view
        returns (bool)
    {
        // @TODO: Add a check for TWAP comparison on 2.5%
        // if TWAP exists for the token.

        if (heartBeat[_tokenAddress] == 0) {
            revert HeartBeatNotSet();
        }

        uint80 latestRoundId = getLatestRoundId(
            _tokenAddress
        );

        uint256 upd = _getRoundTimestamp(
            _tokenAddress,
            latestRoundId
        );

        unchecked {
            upd = block.timestamp < upd
                ? block.timestamp
                : block.timestamp - upd;

            return upd > heartBeat[_tokenAddress];
        }
    }
c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Invalid