Cyfrin / 2023-07-foundry-defi-stablecoin

37 stars 32 forks source link

Block stuffing the chainlink heartbeat transactions for a token will stop any liquidations for users with such collateral on chains with cheap gas #1151

Closed codehawks-bot closed 1 year ago

codehawks-bot commented 1 year ago

Block stuffing the chainlink heartbeat transactions for a token will stop any liquidations for users with such collateral on chains with cheap gas

Severity

Medium Risk

Relevant GitHub Links

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

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L229

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L340

Summary

Block stuffing the chainlink heartbeat transactions for a token that is used as collateral for minting DSC tokens will stop any liquidations for users using such collateral.

Vulnerability Details

The protocol is using chainlink as a source of reliable value feeds for the tokens it is accepting as collateral. The issue arises because staleCheckLatestRoundData() in OracleLib.sol reverts to stale data.

The issue might arise when this protocol gets deployed on a chain with low gas fees and a relatively small block gas limit (e.g. Avalanche, Arbitrum, Optimism). For example, a block's worth of gas on them costs just around 5.5$ at standard prices. Of course, the price will rise if we try to buy out the block and there will probably be other accounts competing with us in including their transaction in that particular block, but the point is still the same.

The way the scenario will play out is when a transaction with the latest oracle data gets submitted to be executed we frontrun it and buy out the rest of the block's gas so the transaction cannot get included in it. Of course, the chainlink transaction will eventually get included. Still, until then there will be a few blocks with reverts in all liquidations of users that use the particular token's price feed. Those delayed liquidations can cause a wide array of issues for the protocol from a few reverts in users calling liquidate() to even insolvency depending on how big the difference between the new prices and the "old” ones is. Such a scenario can cause big issues in scenarios like the USDC price depeg from a few months ago where the token lost >20% worth of value in just a few hours.

Impact

The above scenario can cause anything from a few blocks of delay between the supposed liquidation and the actual time of happening, and even protocol insolvency based on how much time the transaction stuffing continues.

Tools Used

Manual Review + Gummy Bears

Recommendations

There isn't any concrete solution for the above issue other than keeping the liquidation thresholds as conservative as can be so no such scenario can cause a great amount of damage.