-
Notifications
You must be signed in to change notification settings - Fork 1
minhtrng - Oracle requests dont check if latest provider is still active #169
minhtrng - Oracle requests dont check if latest provider is still active #169
Comments
2 comment(s) were left on this issue during the judging contest. 141345 commented:
panprog commented:
|
Escalate After reviewing other watsons comments, agreed that it is a duplicate of #42 with same root cause but slight different impact described |
You've created a valid escalation! To remove the escalation from consideration: Delete your comment. You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final. |
Escalate I want to add more details. This issue description is somewhat vague and hard to actually understand what it means. But from what I understand, after the new oracle provider is set:
This is actually expected behavior and at most I see that it will request from both oracles, but will only use data from 1 oracle. For example: once provider1 request is commited and provider2 request is commited, provider1.latest() will still be used and provider2.latest() will be ignored until provider2.latest().timestamp > last provider1 request timestamp (100). This is the only issue I see: new provider will be requested, but the commited price for that request will be ignored (previous provider price used instead). There is no impact in this and there is no funds loss in keeper fees - both providers will receive the fee for the keeper. So the issue should be invalid or low. The report might also imply that in such case provider1 will keep returning latest() forever, but it's not true: if provider1 last request is already commited, then it will switch to provider2 immediately after provider2 latest().timestamp is higher than provider1 last request timestamp. If provider1 last request is not commited yet, then it just has to wait until keepers commit this last request and it will then automatically switch to provider2. Either way, the report is either low or invalid. |
You've created a valid escalation! To remove the escalation from consideration: Delete your comment. You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final. |
The issue assumes that the granularity is not in effect and price updates are meant to happen multiple times within an epoch (which reflects the state of the code in scope) So the issue describes the following: Epoch goes from time 100 to 200:
However, I admit that with the fix of #42 this is a non-issue. Due to this same root-cause enabling this issue, I would like to ask this issue to be marked as duplicate of #42. this shouldnt change anything regarding payouts, since I already have submitted a duplicate of it, this only affects my valid-invalid-ratio |
Oh! Now I understand. So is it correct that what you mean is: t=50: After t=100, provider1 is commited to requested timestamp=50, so provider1.latest().timestamp = 50 Now oracle.latest() will always return provider1.latest() because if (uint256(oracles[global.latest].timestamp) > latestTimestamp) return false; oracles[1].timestamp = 100 (last requested timestamp) So at this time oracle will keep returning stale price until So while this issue is fixable by anyone (just commit unrequested once), it's unexpected and requires manual intervention to correctly switch the provider. Hm.. I think this issue core reason is indeed #42, so it's basically #42 with a different (medium) impact, should be duplicate of it. |
suggestion: Since with the fix of #42 this one will be no problem. |
Extra context: this may be a duplicate of #42, as our fix for that one does cover most of this issue as well. We've posted a separate fix here for the specific issue of double-requesting on both the latest and current oracle during the small period of an oracle update. This is a minor issue, but since there is an economic cost to requesting, it's not technically correct to request both in the old and new sub-oracles for the same timestamp. |
Escalations have been resolved successfully! Escalation status:
|
minhtrng
medium
Oracle requests dont check if latest provider is still active
Summary
Requests in the oracle will always request from the
current
provider even if thelatest
provider is still active. This can cause a gap in price, whenlatest
provider will not be updated but its most recent data will still be used.Vulnerability Detail
Oracle.request
always requests fromglobal.current
:If the
global.latest
is not stale yet, itslatest()
data will be used (but not updated, due to the above):Impact
Stale price being used due to old provider still being active but receiving no udpate for the last granularity window that it is being active.
Code Snippet
https://github.com/sherlock-audit/2023-07-perennial/blob/c2f6141c231d3952898ea47793872cac69a1d2af/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L38
Tool used
Manual Review
Recommendation
Request from
oracles[global.latest].provider
ifblock.timestamp
is not past its ending time yet.Duplicate of #42
The text was updated successfully, but these errors were encountered: