0x52 - chainlinkAdaptor uses the same heartbeat for both feeds which is highly dangerous

[sherlock-admin]

0x52

medium

chainlinkAdaptor uses the same heartbeat for both feeds which is highly dangerous

Summary

chainlinkAdaptor uses the same heartbeat for both feeds when checking if the data feed is fresh. The issue with this is that the USDC/USD oracle has a 24 hour heartbeat, whereas the average has a heartbeat of 1 hour. Since they use the same heartbeat the heartbeat needs to be slower of the two or else the contract would be nonfunctional most of the time. The issue is that it would allow the consumption of potentially very stale data from the non-USDC feed.

Vulnerability Detail

See summary

Impact

Either near constant downtime or insufficient staleness checks

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/adaptor/chainlinkAdaptor.sol#L43-L55

Tool used

Manual Review

Recommendation

Use two separate heartbeat periods

[JoscelynFarr]

The contract are trying to get the latest price in here:https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/adaptor/chainlinkAdaptor.sol#LL47C1-L47C1

And the heartbeat is trying to prevent chainlink stop updating. It is the same as chainlink's heartbeat. https://docs.chain.link/data-feeds/price-feeds/addresses/?network=arbitrum#Arbitrum%20Mainnet

[deadrosesxyz]

Escalate for 10 USDC I don’t think the sponsor properly understood the issue. On Arbitrum, as well as pretty much any other network, different token pairs have different heartbeats. If the oracle gets the latest price for two pairs with different heartbeats, using the same heartbeat variable for validation would cause either one of the following:

1. Oracle will be down (will revert) most of the time.
2. Oracle will allow for stale prices

When validating prices for two different token pairs, two different heartbeats must be used.

[sherlock-admin]

Escalate for 10 USDC I don’t think the sponsor properly understood the issue. On Arbitrum, as well as pretty much any other network, different token pairs have different heartbeats. If the oracle gets the latest price for two pairs with different heartbeats, using the same heartbeat variable for validation would cause either one of the following:

1. Oracle will be down (will revert) most of the time.
2. Oracle will allow for stale prices

When validating prices for two different token pairs, two different heartbeats must be used.

You've created a valid escalation for 10 USDC!

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.

[JoscelynFarr]

https://github.com/JOJOexchange/smart-contract-EVM/commit/c4270e0dc4da0db56173e39d8b6318e47999a07d https://github.com/JOJOexchange/JUSDV1/commit/f1699ae81e81eb190914d1c2ae491a825389daac fix

[hrishibhat]

Result: Medium Has duplicates Given that the code uses the same heartbeat to validate both assets, when both assets can have different heartbeats, considering this issue a valid medium

Sponsor comment:

got it, we will accept this issue

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• iamjakethehuman: accepted

[IAm0x52]

Fix looks good. Contract now uses separate heartbeats for asset and USDC