search

sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

stopthecap - No clear threshold on when the oracle is updated will cause stale prices to be accepted #150

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

stopthecap

medium

No clear threshold on when the oracle is updated will cause stale prices to be accepted

Summary

No clear threshold on when the oracle is updated will cause stale prices to be accepted

Vulnerability Detail

According to the team, the oracle is updated depending on the gas fees.

"" the oracle price update is limited by cost of gas fee during updating. we want to have a balance between reaching the true price vs saving the cost. Once we move to cheaper chains like arbitrum or matic, we will update the oracle a lot more frequently just fyi ""

Not having a clear point in time when the oracle will be updated, will cause that in times when the network is very expensive to include transactions, stale prices of assets/stables will be accepted as the current price, causing wrong/stale prices be fetched as if they were the latest.

Impact

Wrong/stale prices will be used in times when gas fees are very expensive

Code Snippet8

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

Tool used

Manual Review

Recommendation

Add a clear threshold when the prices should be updated (ex. each 2 minutes), potentially not using the gas fees as an indicator to when updating the oracle.

SunXrex commented 1 year ago

Fixed: https://github.com/xrex-inc/Unitas-Protocol/pull/9

0xffff11 commented 1 year ago

Fix looks good.

The new implementation includes a default threshold of 1 day and a threshold according to each token that unitas uses, which can be changed with a specific setter function. If a threshold is not added for whatever reason or token, it will return the default 1 day threshold.

If the threshold is less than type(uint32).max , basically always, the stale price will be checked and validated:

 if (threshold < type(uint32).max) {
            uint64 priceTimestamp = prices[asset].timestamp;
            _require(
                priceTimestamp > block.timestamp || block.timestamp - priceTimestamp <= threshold,
                Errors.PRICE_STALE
            );