sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

oxcm - Potential Future Timestamp Manipulation in the XOracle Contract #154

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

oxcm

medium

Potential Future Timestamp Manipulation in the XOracle Contract

Summary

The XOracle contract does not properly validate the timestamp input for the putPrice function. This may allow malicious or compromised feeders to inject future timestamps, effectively preventing price updates until that future time.

Vulnerability Detail

In the putPrice function, there's a requirement that the new timestamp must be greater than the previous timestamp. However, there is no upper limit for this new timestamp. A malicious or compromised account with the FEEDER_ROLE could provide a timestamp far into the future, causing the oracle to be unable to update prices until that future time.

Impact

If this happens, this vulnerability can lead to the oracle being frozen and unable to provide updated price information. This could disrupt any dependent contracts or operations that rely on the Oracle for up-to-date price data.

Code Snippet

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

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);
    }

Tool used

Manual Review

Recommendation

It is recommended to enforce a check that the timestamp provided to the putPrice function must not be greater than the current block timestamp. This can be done by adding a line such as require(block.timestamp >= timestamp, "Timestamp cannot be in the future"); at the start of the putPrice function. This will ensure that the input timestamp is not in the future.

function putPrice(address asset, uint64 timestamp, uint256 price) public onlyRole(FEEDER_ROLE) {
    require(block.timestamp >= timestamp, "Timestamp cannot be in the future");
    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);
}