Skip to content
This repository has been archived by the owner on Oct 1, 2023. It is now read-only.

roguereddwarf - ControllerPeggedAssetV2: outdated price may be used which can lead to wrong depeg events #70

Open
sherlock-admin opened this issue Mar 27, 2023 · 3 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

roguereddwarf

high

ControllerPeggedAssetV2: outdated price may be used which can lead to wrong depeg events

Summary

The updatedAt timestamp in the price feed response is not checked. So outdated prices may be used.

Vulnerability Detail

The following checks are performed for the chainlink price feed:
https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L299-L315

As you can see the updatedAt timestamp is not checked.
So the price may be outdated.

Impact

The price that is used by the Controller can be outdated. This means that a depeg event may be caused due to an outdated price which is incorrect. Only current prices must be used to check for a depeg event.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol#L273-L318

Tool used

Manual Review

Recommendation

Introduce a reasonable limit for how old the price can be and revert if the price is older:

iff --git a/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol b/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol
index 0587c86..cf2dcf5 100644
--- a/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol
+++ b/Earthquake/src/v2/Controllers/ControllerPeggedAssetV2.sol
@@ -275,8 +275,8 @@ contract ControllerPeggedAssetV2 {
             ,
             /*uint80 roundId*/
             int256 answer,
-            uint256 startedAt, /*uint256 updatedAt*/ /*uint80 answeredInRound*/
-            ,
+            uint256 startedAt, 
+            uint256 updatedAt, /*uint80 answeredInRound*/
 
         ) = sequencerUptimeFeed.latestRoundData();
 
@@ -314,6 +314,8 @@ contract ControllerPeggedAssetV2 {
 
         if (answeredInRound < roundID) revert RoundIDOutdated();
 
+        if (updatedAt < block.timestamp - LIMIT) revert PriceOutdated();
+
         return price;
     }
@3xHarry
Copy link

3xHarry commented Apr 5, 2023

considering this

@3xHarry
Copy link

3xHarry commented Apr 11, 2023

fix PR: Y2K-Finance/Earthquake#141

@3xHarry 3xHarry added the Will Fix The sponsor confirmed this issue will be fixed label Apr 28, 2023
@IAm0x52
Copy link
Collaborator

IAm0x52 commented May 5, 2023

Fix looks good. Controller will now revert if price is stale

3xHarry added a commit to Y2K-Finance/Earthquake that referenced this issue May 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants