code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Once observationCardinalityNext reaches MAX_CARDINALITY, the function will no longer be able to increment it, leading to a potential Denial of Service (DoS) condition. #231

Closed c4-bot-9 closed 5 months ago

c4-bot-9 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/PowerFarms/PendlePowerFarmController/PendlePowerFarmToken.sol#L347-L357

Vulnerability details

Impact

The _increaseCardinalityNext function in the PendlePowerFarmToken contract is responsible for increasing the observationCardinalityNext value of the underlying PENDLE_MARKET contract. However, there is a condition that limits this value to MAX_CARDINALITY. Once observationCardinalityNext reaches MAX_CARDINALITY, no new observations can be added to the market, which could potentially lead to stale data being used for calculations. However, this does not seem to cause a Denial-of-Service (DoS) condition, as the contract can still function with the existing observations.

Proof of Concept

The _increaseCardinalityNext function in the PendlePowerFarmToken contract is responsible for increasing the observationCardinalityNext value of the underlying PENDLE_MARKET contract. However, there is a condition that limits this value to MAX_CARDINALITY. Once observationCardinalityNext reaches MAX_CARDINALITY, the function will no longer be able to increment it, leading to a potential Denial of Service (DoS) condition. Here's the relevant code section:

   function _increaseCardinalityNext()
       internal
   {
       MarketStorage memory storageMarket = PENDLE_MARKET._storage();

       if (storageMarket.observationCardinalityNext < MAX_CARDINALITY) {
           PENDLE_MARKET.increaseObservationsCardinalityNext(
               storageMarket.observationCardinalityNext + 1
            );
       }
   }

This function checks if the current observationCardinalityNext value of the PENDLE_MARKET is less than MAX_CARDINALITY. If so, it increments the value by calling the increaseObservationsCardinalityNext function of the PENDLE_MARKET contract. However, once observationCardinalityNext reaches MAX_CARDINALITY, the condition storageMarket.observationCardinalityNext < MAX_CARDINALITY will be false, and the function will no longer increment the value.

Tools Used

Manual

Recommended Mitigation Steps

Consider removing the check against MAX_CARDINALITY and always increment the observationCardinalityNext value. However, this may have other implications that need to be carefully considered. Alternatively, you could increase the MAX_CARDINALITY value to a sufficiently large number that would realistically never be reached during the contract's lifetime. Here's an example of how the _increaseCardinalityNext function could be modified to remove the limit:

   function _increaseCardinalityNext()
       internal
   {
       MarketStorage memory storageMarket = PENDLE_MARKET._storage();
       PENDLE_MARKET.increaseObservationsCardinalityNext(
           storageMarket.observationCardinalityNext + 1
       );
   }

This modification removes the check against MAX_CARDINALITY and simply increments the observationCardinalityNext value without any limitation. However, it's essential to carefully analyze the implications of this change on the overall system and ensure that it does not introduce any other potential issues.

Assessed type

Other

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

GalloDaSballo commented 5 months ago
   if (storageMarket.observationCardinalityNext < MAX_CARDINALITY) {
       PENDLE_MARKET.increaseObservationsCardinalityNext(
           storageMarket.observationCardinalityNext + 1
        );
   }

It will skip the check

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Insufficient proof