Does not validate price freshness when using the oracle price, allowing stale oracle price to be used

Summary

Does not validate price freshness when using the oracle price

Vulnerability Detail

In the current implementation,

the protocol heavily reply on the FEEDER_ROLE to update the price

    function putPrice(address asset, uint64 timestamp, uint256 price) public onlyRole(FEEDER_ROLE) {
        uint64 prev_timestamp = prices[asset].timestamp;
        uint256 prev_price = prices[asset].price;
        require(prev_timestamp < timestamp, "Outdated timestamp");
        prices[asset] = IOracle.Price(asset, timestamp, prev_timestamp, price, prev_price);
        emit newPrice(asset, timestamp, price);
    }

    function updatePrices(IOracle.NewPrice[] calldata _array) external onlyRole(FEEDER_ROLE) {
        uint256 arrLength = _array.length;
        for(uint256 i=0; i<arrLength; ){
            address asset = _array[i].asset;
            uint64 timestamp = _array[i].timestamp;
            uint256 price = _array[i].price;
            putPrice(asset, timestamp, price);
            unchecked {
                i++;
            }
        }
    }

there are two get method:

function getPrice(address asset) public view returns (uint64, uint64, uint256, uint256) {
    return (
        prices[asset].timestamp,
        prices[asset].prev_timestamp,
        prices[asset].price,
        prices[asset].prev_price
    );
}

function getLatestPrice(address asset) public view returns (uint256) {
    return prices[asset].price;
}

then this getLatestPrice is used when swapping and when evaluting the collateral


    function _getSwapResult(
        ITokenManager.PairConfig memory pair,
        address tokenIn,
        address tokenOut,
        AmountType amountType,
        uint256 amount
    )
        internal
        view
        returns (uint256 amountIn, uint256 amountOut, IERC20Token feeToken, uint256 fee, uint24 feeNumerator, uint256 price)
    {
        _checkAmountPositive(amount);

        // Checks the tokens of the pair config are valid
        bool isBuy = tokenOut == pair.baseToken;
        _require(
            (isBuy && tokenIn == pair.quoteToken) ||
                (tokenOut == pair.quoteToken && tokenIn == pair.baseToken),
            Errors.PAIR_INVALID
        );

        address priceQuoteToken = _getPriceQuoteToken(tokenIn, tokenOut);
        price = oracle.getLatestPrice(priceQuoteToken);

note the function call

 price = oracle.getLatestPrice(priceQuoteToken);

the latest price is used, but latest price does not mean the updated price because the timestamp of the price is never validated

Impact

for example, the oracle price is updated one day ago at timestamp 1000, the price of token is 10000 USD, then one day later, the price of token is 5000 USD,

but oracle price is not updated

the getLatestPrice() does not check the price refreshness and still wronlgy use the 10000 USD price level to evalute the swap amount and collateral amount, user can either swap for very discounted rate or overpaying the tokenIn amount!

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/Unitas.sol#L439

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/XOracle.sol#L58

Tool used

Manual Review

Recommendation

We recommend the protocol check the last updated timestamp of the oracle before use the oracle price

Duplicate of #150

sherlock-audit / 2023-04-unitasprotocol-judging

ctf_sec - Does not validate price freshness when using the oracle price, allowing stale oracle price to be used #148