user will not be able to close his position (run out of gas)

Summary

The ConvexCurveLPVault.sol contract allows users to earn a yield on curve token deposits. Rewards are paid out in native CRV and CVX tokens but the reward manager of the base pool may opt to add extra rewards. There is no guarantee that the tokens involved will be efficient in their use of gas, and there are no upper bounds on the number of extra rewards check [here]()

Vulnerability Detail

Because the reward manager has the ability to extend the list of extra rewards, they can extend it such that the closePositionFarm() function is unable to execute within a single block.

to close the farm position you should invoke this function ConvexSpell.closePositionFarm() which has all the following loops 1- in wConvexPools.burn()

        for (uint i = 0; i < extraRewardsCount; i++) {
            address rewarder = IRewarder(crvRewarder).extraRewards(i);
            uint256 stRewardPerShare = accExtPerShare[tokenId][i];
            tokens[i + 1] = IRewarder(rewarder).rewardToken();
            rewards[i + 1] = _getPendingReward(
                stRewardPerShare,
                rewarder,
                amount,
                lpDecimals
            );
        }

and this

        for (uint i = 0; i < rewardTokens.length; i++) {
            IERC20Upgradeable(rewardTokens[i]).safeTransfer(
                msg.sender,
                rewards[i]
            );
        }

2- Swap rewards tokens on UniswapV2

        // 2. Swap rewards tokens to debt token
        for (uint256 i = 0; i < rewardTokens.length; i++) {//?@audit 4.1 - this part will add insult to injury 
            uint256 rewards = _doCutRewardsFee(rewardTokens[i]);
            _ensureApprove(rewardTokens[i], address(swapRouter), rewards);
            swapRouter.swapExactTokensForTokens(
                rewards,
                0,
                swapPath[i],
                address(this),
                type(uint256).max
            );
        }

3- At the end execute() need to invoke isLiquidatable() which has this loop to get all the rewards value from the Oracle

            for (uint256 i; i < tokens.length; i++) {
                rewardsValue += oracle.getTokenValue(tokens[i], rewards[i]);
            }

As a result of all these loops together, the user's positions could only get liquidated

Impact

user will not be able to close his position because attempts to do so will revert (run out of gas)

Code Snippet

Tool used

Manual Review

Recommendation

Consider restricting the number of extra rewards by only iterating through the first X number of tokens

Ch-301 commented 1 year ago

Escalate for 10 USDC

Similar issue on C4 though it has only one loop in my finding, we are playing with 3 loops, so getting to this stage (run out of gas) is much easier

sherlock-admin commented 1 year ago

Escalate for 10 USDC

Similar issue on C4 though it has only one loop in my finding, we are playing with 3 loops, so getting to this stage (run out of gas) is much easier

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.

ctf-sec commented 1 year ago

This is on the verge of low / medium, very unlikely to happen, Intend to leave it as "low severity" for now

hrishibhat commented 1 year ago

Escalation rejected

Valid low Given the conditions and the description of the issue, this is a low severity.