sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

Juntao - Stale price data may be used for swaps #97

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Juntao

medium

Stale price data may be used for swaps

Summary

Protocol doesn't check if the lastest price data from XOracle is outdated, thus stale price may be used for swaps.

Vulnerability Detail

Protocol allows addresses with the FEEDER_ROLE to update the prices of assets. By calling putPrice(...) or updatePrices(...) method, feeders can put the newest price data in prices mapping.

In phrase 1, feeders are EOAs and controlled by protocol team, the price data is updated mannually and there is no fixed update time. It's reasonable to think that manual update is prone to errors and price data may not be updated in time because of (including but not limited to) below causes:

Price data for swaps is obtained from XOracle#getLatestPrice(...), however, this function simply returns the last price data stored in prices mapping and doen't check if the price is outdated:

    return prices[asset].price;

If lastest price data is not updated timely, stale price may be used for swaps, both protocol and users may suffer a loss.

Impact

Stale price being used for swaps may cause loss to protocol and users.

Code Snippet

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

Tool used

Manual Review

Recommendation

Potocol should compare current timestamp with price timestamp to ensure no stale price is used:

+   uint256 private THRESHOLD;
    ...
    function getLatestPrice(address asset) public view returns (uint256) {
+       (uint256 timestamp,,,) = getPrice(asset);
+       require(block.timestamp - timestamp <= THRESHOLD, "Stale price");
        return prices[asset].price;
    }

Duplicate of #150