Foundry DeFi Stablecoin CodeHawks Audit Contest

Summary

The stale period (3 hours) is too large for Ethereum, Polygon, BNB, and Optimism chains, leading to consuming stale price data. On the other hand, that period is too small for Arbitrum and Avalanche chains, rendering the DSC protocol unable to operate.

Vulnerability Details

In the OracleLib library, the TIMEOUT constant is set to 3 hours. In other words, the staleCheckLatestRoundData() would consider the price data fed by Chainlink's price feed aggregators to be stale only after the last update time has elapsed 3 hours.

Since the DSC protocol supports every EVM chain (confirmed by the client), let's consider the ETH / USD oracles on different chains.

• On Ethereum, the oracle will update the price data every ~1 hour.

• On Polygon, the oracle will update the price data every ~25 seconds.

• On BNB (BSC), the oracle will update the price data every ~60 seconds.

• On Optimism, the oracle will update the price data every ~20 minutes.

• On Arbitrum, the oracle will update the price data every ~24 hours.

• On Avalanche, the oracle will update the price data every ~24 hours.

On some chains such as Ethereum, Polygon, BNB, and Optimism, 3 hours can be considered too large for the stale period, causing the staleCheckLatestRoundData() to return stale price data.

Whereas, on some chains, such as Arbitrum and Avalanche, 3 hours is too small. Specifically, if the DSC protocol is deployed to Arbitrum or Avalanche, the protocol will be unable to operate because the "if (secondsSince > TIMEOUT)" condition will be met, causing a transaction to be reverted in the staleCheckLatestRoundData().

• TIMEOUT definition: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/libraries/OracleLib.sol#L19

• Use of TIMEOUT: https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/libraries/OracleLib.sol#L30

Impact

Setting the stale period (TIMEOUT constant) too large could lead to incorrect reporting of prices of collateral tokens. The incorrect prices can cause the DSC protocol's functions (e.g., mintDsc(), burnDsc(), redeemCollateral(), and liquidate()) to operate incorrectly, affecting the protocol's disruption.

On the other hand, setting the stale period too small could render the DSC protocol unable to operate.

Tools Used

Manual Review

Recommendations

Even on the same chain, different collateral tokens can have different heartbeats (the period to update the price data on chain). For instance, the heartbeat for the DAI / USD oracle on Ethereum is ~1 hour, whereas the heartbeat for the USDT / USD oracle on the same chain is ~24 hours.

Thus, I recommend using the mapping data type to record the TIMEOUT parameter of each collateral token and setting each token's TIMEOUT with an appropriate stale period.

Furthermore, I also recommend adding a setter function for updating the stale period of each specific collateral token.