Ch_301 - `CurveSpell.closePositionFarm()` will keep reverting

[sherlock-admin]

Ch_301

high

CurveSpell.closePositionFarm() will keep reverting

Summary

Curve has a weight system for the Gauges https://resources.curve.fi/reward-gauges/understanding-gauges#the-dao

The weight systems allow the Curve DAO to dictate where the CRV inflation should go…

So there is no guarantee that a specific Gauge will always receive CRV (rewards)

Vulnerability Detail

In case the user invokes closePositionFarm() after this line

wCurveGauge.burn(pos.collId, param.amountPosRemove);

on CurveSpell.closePositionFarm() the SPELL will receive no rewards (no CRV) And after CurveSpell.closePositionFarm.[_doCutRewardsFee()](https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/spell/CurveSpell.sol#L165)

  uint256 rewards = _doCutRewardsFee(CRV);

the value of rewards is zero rewards == 0 So swapExactTokensForTokens will execte with amountIn == 0

    function swapExactTokensForTokens(
        uint amountIn,
        uint amountOutMin,
        address[] calldata path,
        address to,
        uint deadline
    ) external virtual override ensure(deadline) returns (uint[] memory amounts) {
        amounts = UniswapV2Library.getAmountsOut(factory, amountIn, path);
        require(amounts[amounts.length - 1] >= amountOutMin, 'UniswapV2Router: INSUFFICIENT_OUTPUT_AMOUNT');
        TransferHelper.safeTransferFrom(
            path[0], msg.sender, UniswapV2Library.pairFor(factory, path[0], path[1]), amounts[0]
        );
        _swap(amounts, path, to);
    }

And this call will revert in this line

Impact

CurveSpell.closePositionFarm() will keep reverting which could lead the position to the liquidation

Code Snippet

        // 1. Take out collateral - Burn wrapped tokens, receive crv lp tokens and harvest CRV
        bank.takeCollateral(param.amountPosRemove);
        wCurveGauge.burn(pos.collId, param.amountPosRemove);

        {
            // 2. Swap rewards tokens to debt token
            uint256 rewards = _doCutRewardsFee(CRV);
            _ensureApprove(CRV, address(swapRouter), rewards);
            swapRouter.swapExactTokensForTokens(
                rewards,
                0,
                swapPath,
                address(this),
                type(uint256).max
            );
        }

Tool used

Manual Review

Recommendation

        // 1. Take out collateral - Burn wrapped tokens, receive crv lp tokens and harvest CRV
        bank.takeCollateral(param.amountPosRemove);
        wCurveGauge.burn(pos.collId, param.amountPosRemove);

        {
            // 2. Swap rewards tokens to debt token
            uint256 rewards = _doCutRewardsFee(CRV);
+            if(rewards > 0){
            _ensureApprove(CRV, address(swapRouter), rewards);
            swapRouter.swapExactTokensForTokens(
                rewards,
                0,
                swapPath,
                address(this),
                type(uint256).max
            );
+            }
        }

[Ch-301]

Escalate for 10 USDC

The Curve docs are clear

The weight systems allow the Curve DAO to dictate where the CRV inflation should go…

there is no guarantee that the gauges will receive CRV because the weight system is controlled by the Curve DAO so at any time, the weight (CRV) of any gauge could down to zero

[sherlock-admin]

Escalate for 10 USDC

The Curve docs are clear

The weight systems allow the Curve DAO to dictate where the CRV inflation should go…

there is no guarantee that the gauges will receive CRV because the weight system is controlled by the Curve DAO so at any time, the weight (CRV) of any gauge could down to zero

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]

Can be a valid medium

[hrishibhat]

Escalation rejected

Valid low Although there is a possible edge case mentioned where it could happen, these gauges are whitelisted by the admin. This would affect only those users who would deposit after the gauge was disabled. the admin can always disable deposits by setting maxPosSize to zero. Given all these factors considering this issue a valid low.

[sherlock-admin]

Escalation rejected

Valid low Although there is a possible edge case mentioned where it could happen, these gauges are whitelisted by the admin. This would affect only those users who would deposit after the gauge was disabled. the admin can always disable deposits by setting maxPosSize to zero. Given all these factors considering this issue a valid low.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.