search

code-423n4 / 2023-01-reserve-findings

4 stars 2 forks source link

Protocol uses unnecessarily large oracle timeout for assets #81

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/utils.ts#L17-L19 https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/phase2-assets/1_deploy_assets.ts#L46 https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/phase2-assets/1_deploy_assets.ts#L60 https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/phase2-assets/2_deploy_collateral.ts#L48 https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/phase2-assets/2_deploy_collateral.ts#L105 https://github.com/reserve-protocol/protocol/blob/master/scripts/deployment/phase2-assets/2_deploy_collateral.ts#L526 https://github.com/reserve-protocol/protocol/blob/master/contracts/plugins/assets/OracleLib.sol#L27

Vulnerability details

Impact

The protocol implements a safety mechanism to guard against stale chainlink feeds. If the oracle's last response is more than a day ago the contract reverts. But, chainlink feeds are refreshed at set intervals (heartbeat). Most of the feeds used by the protocol on deployment are refreshed every hour instead of daily. With the current configuration, a feed that has been stale for 23 rounds (23 hours) will still be considered valid. The prices of assets are of critical value for the protocol. Minimizing the risk of using stale prices is of utmost importance. Setting the timeout to 24 hours for every chainlink feed is an unnecessary risk.

Proof of Concept

The following token feeds are refreshed every hour:

The assets deployed with those feeds set oracleTimeout to 86000s, see links to affected code.

Tools Used

none

Recommended Mitigation Steps

Set the oracleTimeout to a value a little higher than 3600. The oracle is not updated exactly 3600 seconds later so you have to leave a little room for error. For example, the AAVE/USD feed is updated after 3624 seconds in between rounds 55340232221128673944 and 55340232221128673945

c4-judge commented 1 year ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

0xean marked the issue as grade-b

c4-sponsor commented 1 year ago

tbrent marked the issue as sponsor confirmed

tbrent commented 1 year ago

Just fyi we agree this is a good suggestion, we just weren't able to include it in time for the mitigation review.