sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

lemonmon - Potential DOS condition due to exceeding block gas limit when calling `ClearingHouse.settleFunding()` #249

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

lemonmon

medium

Potential DOS condition due to exceeding block gas limit when calling ClearingHouse.settleFunding()

Summary

With a rising number of supported markets, ClearingHouse.settleFunding() can potentially reach a point, where it exceeds the block gas limit on Avalanche C-Chain, leading to a potential DOS condition.

Vulnerability Detail

First ClearingHouse.settleFunding() is called, which iterates over all markets, calling amms[i].settleFunding() for each market:

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/ClearingHouse.sol#L260-L267

Then inside AMM.settleFunding() the underlying twap price is determined:

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L255

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L298-L299

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/Oracle.sol#L76

Especially inside the oracle code (line 76) there is a gas expensive while (true) that might exceed the block gas limit, when there are many markets for which the unterlying twap price has to be fetched.

Note: A gas issue with ClearingHouse.settleFunding() was already reported in a previous audit, where the function Amm._calcTwap back then caused a very high gas consumption according to the profiler. The Amm._calcTwap() and it's while (true) loop functionality, which is very gas intensive, was then refactored into Oracle.getUnderlyingTwapPrice() (line 76 Oracle.sol), where the code is very identical and the potential excess of available block gas will still occur if many markets are being supported.

Impact

Potential DOS condition when calling ClearingHouse.settleFunding().

Code Snippet

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/ClearingHouse.sol#L260-L267

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L255

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L298-L299

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/Oracle.sol#L76

Tool used

Manual Review

Recommendation

Consider adding functionality to settle funding per market or per an array of markets, to mitigate exceeding the block gas limit when having a high number of supported markets.