ShadowForce - unbounded loop in view function leads to a dos

[sherlock-admin]

ShadowForce

medium

unbounded loop in view function leads to a dos

Summary

unbounded loops in getRolloverTVL and getDepositQueueTVL functions will lead to a DOS

Vulnerability Detail

In the carousal contract we can see there are two unbounded loops. Because the rolloverQueue and depostiQueue are infinitely large, the function may encounter an Out of gas problem. the DOS issue can block external protocol integration that relies on these two functions to accurately retrieve the protocol TVL and also DOS the frontend website that relies on calling this function to display the protocol TVL.
the two unbounded loops can be viewed below.

  function getRolloverTVL(uint256 _epochId)
        public
        view
        returns (uint256 tvl)
    {
        for (uint256 i = 0; i < rolloverQueue.length; i++) {
            if (
                rolloverQueue[i].epochId == _epochId &&
                (previewWithdraw(
                    rolloverQueue[i].epochId,
                    rolloverQueue[i].assets
                ) > rolloverQueue[i].assets)
            ) {
                tvl += rolloverQueue[i].assets;
            }
        }
    }

    /** @notice returns the total value locked in the deposit queue
     * @return tvl total value locked in the deposit queue
     */
    function getDepositQueueTVL() public view returns (uint256 tvl) {
        for (uint256 i = 0; i < depositQueue.length; i++) {
            tvl += depositQueue[i].assets;
        }
    }

Impact

Because rolloverQueue and depositQueue can be infinitely large. These view functions wich are vital to the protocol may run out of gas and not work entirely leading to a Denial Of Service. Users funds will be locked up in queue, this is a loss of potential funds.

Code Snippet

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/Carousel/Carousel.sol#L655-L671

https://github.com/Y2K-Finance/Earthquake/blob/736b2e1e51bef6daa6a5ecd1decb7d156316d795/src/v2/Carousel/Carousel.sol#L687-L694

Tool used

Manual Review

Recommendation

add running total or an accumulator variable and update the total TVL when request is added or removed to avoid unbounded loop.

[twicek]

Escalate for 10 USDC

This issue is not a duplicate of #174 and should not be medium severity since the issue is located in a view function that is not used in any state changing function.

As per the documentation:

• Incorrect values in View functions are by default considered low.
  Exception: In case any of these incorrect values returned by the view functions are used as a part of a larger function which would result in loss of funds then it would be a valid medium/high depending on the impact.

[sherlock-admin]

Escalate for 10 USDC

This issue is not a duplicate of #174 and should not be medium severity since the issue is located in a view function that is not used in any state changing function.

As per the documentation:

• Incorrect values in View functions are by default considered low.
  Exception: In case any of these incorrect values returned by the view functions are used as a part of a larger function which would result in loss of funds then it would be a valid medium/high depending on the impact.

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.

[hrishibhat]

Escalation accepted

Valid low
Not a duplicate of #174

[sherlock-admin]

Escalation accepted

Valid low
Not a duplicate of #174

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.